CODEWITHSUNDEEP
ruby-on-railsoauthruby-style-guide
Chris Hough
Chris Hough

Reputation: 3558

How to improve on this Oauth Rubocop code?

I have the following method setup to assist with refreshing Oauth tokens:

module WhiplashOmniAuthentication
  extend ActiveSupport::Concern

  module ClassMethods
    def from_omniauth(auth)
      Rails.logger.debug auth.inspect
      where(provider: auth.provider, uid: auth.uid).first_or_create do |user|
        user.provider = auth.provider
        user.uid = auth.uid
        user.email = auth.info.email
        user.store_token(auth.credentials)
      end
    end
  end

  def refresh_token!
    settings = Devise.omniauth_configs[:whiplash].strategy
    strategy = OmniAuth::Strategies::Whiplash.new(nil, settings.client_id, settings.client_secret, client_options: settings.client_options)
    client = strategy.client
    access_token = OAuth2::AccessToken.new client, token, refresh_token: refresh_token
    if access_token
      begin
        result = access_token.refresh!
        store_token(result)
        save
      rescue OAuth2::Error => e
        errors[:token] << e.inspect
        return false
      end
    else
      errors[:token] << e.inspect
      return false
    end
  end

  def store_token(auth_token)
    self.token = auth_token.token
    self.refresh_token = auth_token.refresh_token
    self.token_expires_at = Time.at(auth_token.expires_at).to_datetime
  end

  def token_expired?
    Time.now > token_expires_at
  end
end

I tried breaking this out into separate methods but it keeps blowing up, so I am going to defer to readers here. I am looking for recommendations to pass the cops and learning.

Upvotes: 0

Views: 49

Answers (1)

Dharam Gollapudi
Dharam Gollapudi

Reputation: 6438

You definitely have too many things going on in the refresh_token! implementation. You always want to keep methods to do one thing and only one thing. It makes it easier for testing (for ex: stub out a particular method), debugging and readability.

See if the following helps:

module WhiplashOmniAuthentication
  extend ActiveSupport::Concern

  module ClassMethods
    def from_omniauth(auth)
      Rails.logger.debug auth.inspect
      where(provider: auth.provider, uid: auth.uid).first_or_create do |user|
        user.provider = auth.provider
        user.uid = auth.uid
        user.email = auth.info.email
        user.store_token(auth.credentials)
      end
    end
  end

  def refresh_token!
    access_token ? refresh_access_token! : false
  end

  def refresh_access_token!
    result = access_token.refresh!
    store_token(result)
    save
  rescue OAuth2::Error
    false
  end

  def settings
    @settings ||= Devise.omniauth_configs[:whiplash].strategy
  end

  def strategy
    @strategy ||= OmniAuth::Strategies::Whiplash.new(nil, settings.client_id, settings.client_secret, client_options: settings.client_options)
  end

  def client
    @client ||= strategy.client
  end

  def access_token
    OAuth2::AccessToken.new(client, token, refresh_token: refresh_token)
  end

  def store_token(auth_token)
    self.token = auth_token.token
    self.refresh_token = auth_token.refresh_token
    self.token_expires_at = Time.at(auth_token.expires_at).to_datetime
  end

  def token_expired?
    Time.now > token_expires_at
  end
end

Upvotes: 1

Related Questions