Search code examples
ruby-on-railsoauthruby-style-guide

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.


Solution

  • 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