publiclab/plots2

View on GitHub
app/controllers/user_sessions_controller.rb

Summary

Maintainability
F
4 days
Test Coverage

Possible unprotected redirect
Open

                  redirect_to params[:return_to] + hash_params

Unvalidated redirects and forwards are #10 on the OWASP Top Ten.

Redirects which rely on user-supplied values can be used to "spoof" websites or hide malicious links in otherwise harmless-looking URLs. They can also allow access to restricted areas of a site if the destination is not validated.

Brakeman will raise warnings whenever redirect_to appears to be used with a user-supplied value that may allow them to change the :host option.

For example,

redirect_to params.merge(:action => :home)

will create a warning like

Possible unprotected redirect near line 46: redirect_to(params)

This is because params could contain :host => 'evilsite.com' which would redirect away from your site and to a malicious site.

If the first argument to redirect_to is a hash, then adding :only_path => true will limit the redirect to the current host. Another option is to specify the host explicitly.

redirect_to params.merge(:only_path => true)

redirect_to params.merge(:host => 'myhost.com')

If the first argument is a string, then it is possible to parse the string and extract the path:

redirect_to URI.parse(some_url).path

If the URL does not contain a protocol (e.g., http://), then you will probably get unexpected results, as redirect_to will prepend the current host name and a protocol.

Possible unprotected redirect
Open

                    redirect_to return_to

Unvalidated redirects and forwards are #10 on the OWASP Top Ten.

Redirects which rely on user-supplied values can be used to "spoof" websites or hide malicious links in otherwise harmless-looking URLs. They can also allow access to restricted areas of a site if the destination is not validated.

Brakeman will raise warnings whenever redirect_to appears to be used with a user-supplied value that may allow them to change the :host option.

For example,

redirect_to params.merge(:action => :home)

will create a warning like

Possible unprotected redirect near line 46: redirect_to(params)

This is because params could contain :host => 'evilsite.com' which would redirect away from your site and to a malicious site.

If the first argument to redirect_to is a hash, then adding :only_path => true will limit the redirect to the current host. Another option is to specify the host explicitly.

redirect_to params.merge(:only_path => true)

redirect_to params.merge(:host => 'myhost.com')

If the first argument is a string, then it is possible to parse the string and extract the path:

redirect_to URI.parse(some_url).path

If the URL does not contain a protocol (e.g., http://), then you will probably get unexpected results, as redirect_to will prepend the current host name and a protocol.

Method handle_site_login_flow has a Cognitive Complexity of 77 (exceeds 5 allowed). Consider refactoring.
Open

  def handle_site_login_flow
    username = params[:user_session][:username] if params[:user_session]
    u = User.find_by(username: username) || User.find_by(email: username)
    if u && u.password_checker != 0
      n = u.password_checker
Severity: Minor
Found in app/controllers/user_sessions_controller.rb - About 1 day to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Method handle_social_login_flow has a Cognitive Complexity of 37 (exceeds 5 allowed). Consider refactoring.
Open

  def handle_social_login_flow(auth)
    # Find an identity here
    @identity = UserTag.find_with_omniauth(auth)
    return_to = request.env['omniauth.origin'] || root_url
    return_to += '?_=' + Time.now.to_i.to_s
Severity: Minor
Found in app/controllers/user_sessions_controller.rb - About 5 hrs to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Method subscribe_multiple_tag has a Cognitive Complexity of 27 (exceeds 5 allowed). Consider refactoring.
Open

  def subscribe_multiple_tag(tag_list)
    if !tag_list || tag_list == ''
      flash[:notice] = "Please enter tags for subscription in the url."
    else
      if tag_list.is_a? String
Severity: Minor
Found in app/controllers/user_sessions_controller.rb - About 3 hrs to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Method handle_site_login_flow has 78 lines of code (exceeds 25 allowed). Consider refactoring.
Open

  def handle_site_login_flow
    username = params[:user_session][:username] if params[:user_session]
    u = User.find_by(username: username) || User.find_by(email: username)
    if u && u.password_checker != 0
      n = u.password_checker
Severity: Major
Found in app/controllers/user_sessions_controller.rb - About 3 hrs to fix

Method handle_social_login_flow has 77 lines of code (exceeds 25 allowed). Consider refactoring.
Open

  def handle_social_login_flow(auth)
    # Find an identity here
    @identity = UserTag.find_with_omniauth(auth)
    return_to = request.env['omniauth.origin'] || root_url
    return_to += '?_=' + Time.now.to_i.to_s
Severity: Major
Found in app/controllers/user_sessions_controller.rb - About 3 hrs to fix

Method subscribe_multiple_tag has 38 lines of code (exceeds 25 allowed). Consider refactoring.
Open

  def subscribe_multiple_tag(tag_list)
    if !tag_list || tag_list == ''
      flash[:notice] = "Please enter tags for subscription in the url."
    else
      if tag_list.is_a? String
Severity: Minor
Found in app/controllers/user_sessions_controller.rb - About 1 hr to fix

Avoid deeply nested control flow statements.
Open

              unless params[:hash_params].to_s.empty?
                hash_params = URI.parse("#" + params[:hash_params]).to_s
              end
Severity: Major
Found in app/controllers/user_sessions_controller.rb - About 45 mins to fix

Avoid deeply nested control flow statements.
Open

            params[:user_session][:openid_identifier] = 'https://old.publiclab.org/people/' + username + '/identity' if username
Severity: Major
Found in app/controllers/user_sessions_controller.rb - About 45 mins to fix

Avoid deeply nested control flow statements.
Open

              if current_user.crypted_password.nil? # the user has not created a pwd in the new site
                flash[:warning] = I18n.t('user_sessions_controller.create_password_for_new_site')
                redirect_to '/profile/edit'
              else
                flash[:notice] = I18n.t('user_sessions_controller.logged_in')
Severity: Major
Found in app/controllers/user_sessions_controller.rb - About 45 mins to fix

Avoid deeply nested control flow statements.
Open

            if session[:openid_return_to] # for openid login, redirects back to openid auth process
              return_to = session[:openid_return_to]
              session[:openid_return_to] = nil
              redirect_to return_to + hash_params
            elsif params[:return_to] && params[:return_to].split('/')[0..3] == ["", "subscribe", "multiple", "tag"]
Severity: Major
Found in app/controllers/user_sessions_controller.rb - About 45 mins to fix

Avoid deeply nested control flow statements.
Open

            if session[:openid_return_to] # for openid login, redirects back to openid auth process
              return_to = session[:openid_return_to]
              session[:openid_return_to] = nil
              redirect_to return_to + hash_params
            else
Severity: Major
Found in app/controllers/user_sessions_controller.rb - About 45 mins to fix

Avoid deeply nested control flow statements.
Open

            PasswordResetMailer.reset_notify(user, key).deliver_later unless user.nil? # respond the same to both successes and failures; security
Severity: Major
Found in app/controllers/user_sessions_controller.rb - About 45 mins to fix

Avoid more than 6 levels of block nesting.
Open

                  if params[:return_to]
                    # params[:return_to] == /login?return_to=/subscribe/multiple/tag/tag1,tag299 ? true
                    return_to = '/' + params[:return_to].split('/')[2..-1].join('/') #== /subscribe/multiple/tag/tag1,tag299
                    redirect_to return_to
                  else

This cop checks for excessive nesting of conditional and looping constructs.

You can configure if blocks are considered using the CountBlocks option. When set to false (the default) blocks are not counted towards the nesting level. Set to true to count blocks as well.

The maximum level of nesting allowed is configurable.

Similar blocks of code found in 2 locations. Consider refactoring.
Open

  def subscribe_multiple_tag(tag_list)
    if !tag_list || tag_list == ''
      flash[:notice] = "Please enter tags for subscription in the url."
    else
      if tag_list.is_a? String
Severity: Major
Found in app/controllers/user_sessions_controller.rb and 1 other location - About 3 hrs to fix
app/controllers/users_controller.rb on lines 438..481

Duplicated Code

Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).

Tuning

This issue has a mass of 124.

We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.

The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.

If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.

See codeclimate-duplication's documentation for more information about tuning the mass threshold in your .codeclimate.yml.

Refactorings

Further Reading

rescue at 255, 12 is not aligned with end at 259, 10.
Open

            rescue ActiveRecord::RecordInvalid

This cop checks whether the rescue and ensure keywords are aligned properly.

Example:

# bad
begin
  something
  rescue
  puts 'error'
end

# good
begin
  something
rescue
  puts 'error'
end

Use safe navigation (&.) instead of checking if an object exists before calling the method.
Open

            elsif params[:return_to] && params[:return_to].split('/')[0..3] == ["", "subscribe", "multiple", "tag"]

This cop transforms usages of a method call safeguarded by a non nil check for the variable whose method is being called to safe navigation (&.).

Configuration option: ConvertCodeThatCanStartToReturnNil The default for this is false. When configured to true, this will check for code in the format !foo.nil? && foo.bar. As it is written, the return of this code is limited to false and whatever the return of the method is. If this is converted to safe navigation, foo&.bar can start returning nil as well as what the method returns.

Example:

# bad
foo.bar if foo
foo.bar(param1, param2) if foo
foo.bar { |e| e.something } if foo
foo.bar(param) { |e| e.something } if foo

foo.bar if !foo.nil?
foo.bar unless !foo
foo.bar unless foo.nil?

foo && foo.bar
foo && foo.bar(param1, param2)
foo && foo.bar { |e| e.something }
foo && foo.bar(param) { |e| e.something }

# good
foo&.bar
foo&.bar(param1, param2)
foo&.bar { |e| e.something }
foo&.bar(param) { |e| e.something }

foo.nil? || foo.bar
!foo || foo.bar

# Methods that `nil` will `respond_to?` should not be converted to
# use safe navigation
foo.to_i if foo

Incorrect indentation detected (column 8 instead of 10).
Open

        # Successfully we have added subscription

This cops checks the indentation of comments.

Example:

# bad
  # comment here
def method_name
end

  # comment here
a = 'hello'

# yet another comment
  if true
    true
  end

# good
# comment here
def method_name
end

# comment here
a = 'hello'

# yet another comment
if true
  true
end

Use 2 (not 0) spaces for indentation.
Open

            flash[:error] = tag.errors.full_messages

This cops checks for indentation that doesn't use the specified number of spaces.

See also the IndentationConsistency cop which is the companion to this one.

Example:

# bad
class A
 def test
  puts 'hello'
 end
end

# good
class A
  def test
    puts 'hello'
  end
end

Example: IgnoredPatterns: ['^\s*module']

# bad
module A
class B
  def test
  puts 'hello'
  end
end
end

# good
module A
class B
  def test
    puts 'hello'
  end
end
end

There are no issues that match your filters.

Category
Status