Strava OAuth revamp.

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2018-08-22 08:43:52 -0400 using RuboCop version 0.49.1.
+# on 2018-10-27 17:28:39 -0400 using RuboCop version 0.49.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new

slack-strava/api/endpoints/slack_endpoint.rb

@@ -76,6 +76,8 @@ class SlackEndpoint < Grape::API
             club = Club.create!(
               strava_club.merge(
                 access_token: user.access_token,
+                refresh_token: user.refresh_token,
+                token_expires_at: user.token_expires_at,
                 token_type: user.token_type,
                 team: user.team,
                 channel_id: channel_id,

slack-strava/models.rb

@@ -1,5 +1,6 @@
 require 'slack-strava/models/error'
 require 'slack-strava/models/brag'
+require 'slack-strava/models/strava_tokens'
 require 'slack-strava/models/array_validator'
 require 'slack-strava/models/activity_fields'
 require 'slack-strava/models/map_types'

slack-strava/models/club.rb

@@ -1,6 +1,7 @@
 class Club
   include Mongoid::Document
   include Mongoid::Timestamps
+  include StravaTokens
   include Brag
 
   field :strava_id, type: String
@@ -14,9 +15,6 @@ class Club
   field :url, type: String
   field :member_count, type: Integer
 
-  field :access_token, type: String
-  field :token_type, type: String
-
   belongs_to :team
   field :channel_id, type: String
   field :channel_name, type: String
@@ -134,9 +132,4 @@ def handle_strava_error(e)
     end
     raise e
   end
-
-  def strava_client
-    raise 'Missing access_token' unless access_token
-    @strava_client ||= Strava::Api::V3::Client.new(access_token: access_token)
-  end
 end

slack-strava/models/strava_tokens.rb

@@ -0,0 +1,62 @@
+module StravaTokens
+  extend ActiveSupport::Concern
+
+  included do
+    field :access_token, type: String
+    field :token_type, type: String
+    field :refresh_token, type: String
+    field :token_expires_at, type: DateTime
+  end
+
+  def get_access_token!(code)
+    args = {
+      client_id: ENV['STRAVA_CLIENT_ID'],
+      client_secret: ENV['STRAVA_CLIENT_SECRET'],
+      grant_type: 'authorization_code',
+      code: code
+    }
+
+    response = HTTMultiParty.public_send(
+      'post',
+      Strava::Api::V3::Configuration::DEFAULT_AUTH_ENDPOINT,
+      query: args
+    )
+
+    raise Strava::Api::V3::ServerError.new(response.code.to_i, response.body) unless response.success?
+
+    response
+  end
+
+  def refresh_access_token!
+    return if token_expires_at && Time.now + 1.hour < token_expires_at
+
+    args = {
+      client_id: ENV['STRAVA_CLIENT_ID'],
+      client_secret: ENV['STRAVA_CLIENT_SECRET'],
+      grant_type: 'refresh_token',
+      refresh_token: refresh_token || access_token # TODO: remove access_token after migration
+    }
+
+    response = HTTMultiParty.post(
+      Strava::Api::V3::Configuration::DEFAULT_AUTH_ENDPOINT,
+      query: args
+    )
+
+    raise Strava::Api::V3::ServerError.new(response.code.to_i, response.body) unless response.success?
+
+    update_attributes!(
+      token_type: response['token_type'],
+      access_token: response['access_token'],
+      refresh_token: response['refresh_token'],
+      token_expires_at: Time.at(response['expires_at'])
+    )
+  end
+
+  def strava_client
+    @strava_client ||= begin
+      refresh_access_token!
+      raise 'Missing access_token' unless access_token
+      Strava::Api::V3::Client.new(access_token: access_token)
+    end
+  end
+end

slack-strava/models/user.rb

@@ -1,12 +1,11 @@
 class User
   include Mongoid::Document
   include Mongoid::Timestamps
+  include StravaTokens
   include Brag
 
   field :user_id, type: String
   field :user_name, type: String
-  field :access_token, type: String
-  field :token_type, type: String
   field :activities_at, type: DateTime
   field :connected_to_strava_at, type: DateTime
   field :is_bot, type: Boolean
@@ -30,7 +29,7 @@ def connected_to_strava?
 
   def connect_to_strava_url
     redirect_uri = "#{SlackStrava::Service.url}/connect"
-    "https://www.strava.com/oauth/authorize?client_id=#{ENV['STRAVA_CLIENT_ID']}&redirect_uri=#{redirect_uri}&response_type=code&scope=view_private&state=#{id}"
+    "https://www.strava.com/oauth/authorize?client_id=#{ENV['STRAVA_CLIENT_ID']}&redirect_uri=#{redirect_uri}&response_type=code&scope=activity:read_all&state=#{id}"
   end
 
   def slack_mention
@@ -78,10 +77,15 @@ def to_s
   end
 
   def connect!(code)
-    response = Strava::Api::V3::Auth.retrieve_access(ENV['STRAVA_CLIENT_ID'], ENV['STRAVA_CLIENT_SECRET'], code)
-    raise "Strava returned #{response.code}: #{response.body}" unless response.success?
+    response = get_access_token!(code)
     create_athlete(Athlete.attrs_from_strava(response['athlete']))
-    update_attributes!(token_type: response['token_type'], access_token: response['access_token'], connected_to_strava_at: DateTime.now.utc)
+    update_attributes!(
+      token_type: response['token_type'],
+      access_token: response['access_token'],
+      refresh_token: response['refresh_token'],
+      token_expires_at: Time.at(response['expires_at']),
+      connected_to_strava_at: DateTime.now.utc
+    )
     logger.info "Connected team=#{team_id}, user=#{user_name}, user_id=#{id}, athlete_id=#{athlete.athlete_id}"
     dm!(text: "Your Strava account has been successfully connected.\nI won't post any private activities, DM me `set private on` to toggle that and `help` for other options.")
     inform!(text: "New Strava account connected for #{slack_mention}.")
@@ -127,11 +131,6 @@ def brag!
     end
   end
 
-  def strava_client
-    raise 'Missing access_token' unless access_token
-    @strava_client ||= Strava::Api::V3::Client.new(access_token: access_token)
-  end
-
   def sync_new_strava_activities!
     sync_strava_activities!(after: activities_at || latest_activity_start_date || connected_to_strava_at || created_at)
   end

spec/api/endpoints/slack_endpoint_spec.rb

@@ -10,7 +10,7 @@
       ENV['SLACK_VERIFICATION_TOKEN'] = token
     end
     context 'interactive buttons' do
-      let(:user) { Fabricate(:user, team: team, access_token: 'token') }
+      let(:user) { Fabricate(:user, team: team, access_token: 'token', token_expires_at: Time.now + 1.day) }
       context 'without a club' do
         let(:club) do
           Club.new(
@@ -131,7 +131,7 @@
           end
         end
         context 'connected user' do
-          let(:user) { Fabricate(:user, team: team, access_token: 'token') }
+          let(:user) { Fabricate(:user, team: team, access_token: 'token', token_expires_at: Time.now + 1.day) }
           let(:nyrr_club) do
             Club.new(
               strava_id: '108605',

spec/models/club_spec.rb

@@ -2,7 +2,7 @@
 
 describe Club do
   let(:team) { Fabricate(:team) }
-  let!(:club) { Fabricate(:club, team: team, strava_id: '43749', access_token: 'token', token_type: 'Bearer') }
+  let!(:club) { Fabricate(:club, team: team, strava_id: '43749', access_token: 'token', token_expires_at: Time.now + 1.day, token_type: 'Bearer') }
   context 'sync_last_strava_activity!', vcr: { allow_playback_repeats: true, cassette_name: 'strava/club_sync_last_strava_activity' } do
     it 'retrieves the last activity' do
       expect {
@@ -29,6 +29,30 @@
       expect { club.sync_last_strava_activity! }.to raise_error Strava::Api::V3::ClientError
       expect(club.access_token).to be nil
     end
+    context 'without a refresh token (until October 2019)', vcr: { cassette_name: 'strava/refresh_access_token' } do
+      before do
+        club.update_attributes!(refresh_token: nil, token_expires_at: nil)
+      end
+      it 'refreshes access token using access token' do
+        club.send(:strava_client)
+        expect(club.refresh_token).to eq 'updated-refresh-token'
+        expect(club.access_token).to eq 'updated-access-token'
+        expect(club.token_expires_at).to_not be_nil
+        expect(club.token_type).to eq 'Bearer'
+      end
+    end
+    context 'with an expired refresh token', vcr: { cassette_name: 'strava/refresh_access_token' } do
+      before do
+        club.update_attributes!(refresh_token: 'refresh_token', token_expires_at: nil)
+      end
+      it 'refreshes access token' do
+        club.send(:strava_client)
+        expect(club.refresh_token).to eq 'updated-refresh-token'
+        expect(club.access_token).to eq 'updated-access-token'
+        expect(club.token_expires_at).to_not be_nil
+        expect(club.token_type).to eq 'Bearer'
+      end
+    end
   end
   context 'brag!' do
     let!(:activity) { Fabricate(:club_activity, club: club) }

spec/models/user_spec.rb

@@ -54,7 +54,7 @@
   end
   context 'sync_new_strava_activities!' do
     context 'recent created_at', vcr: { cassette_name: 'strava/user_sync_new_strava_activities' } do
-      let!(:user) { Fabricate(:user, created_at: DateTime.new(2018, 3, 26), access_token: 'token', token_type: 'Bearer') }
+      let!(:user) { Fabricate(:user, created_at: DateTime.new(2018, 3, 26), access_token: 'token', token_expires_at: Time.now + 1.day, token_type: 'Bearer') }
       it 'retrieves new activities since created_at' do
         expect {
           user.sync_new_strava_activities!
@@ -103,25 +103,49 @@
           user.sync_new_strava_activities!
         end
       end
+      context 'without a refresh token (until October 2019)', vcr: { cassette_name: 'strava/refresh_access_token' } do
+        before do
+          user.update_attributes!(refresh_token: nil, token_expires_at: nil)
+        end
+        it 'refreshes access token using access token' do
+          user.send(:strava_client)
+          expect(user.refresh_token).to eq 'updated-refresh-token'
+          expect(user.access_token).to eq 'updated-access-token'
+          expect(user.token_expires_at).to_not be_nil
+          expect(user.token_type).to eq 'Bearer'
+        end
+      end
+      context 'with an expired refresh token', vcr: { cassette_name: 'strava/refresh_access_token' } do
+        before do
+          user.update_attributes!(refresh_token: 'refresh_token', token_expires_at: nil)
+        end
+        it 'refreshes access token' do
+          user.send(:strava_client)
+          expect(user.refresh_token).to eq 'updated-refresh-token'
+          expect(user.access_token).to eq 'updated-access-token'
+          expect(user.token_expires_at).to_not be_nil
+          expect(user.token_type).to eq 'Bearer'
+        end
+      end
     end
     context 'old created_at' do
-      let!(:user) { Fabricate(:user, created_at: DateTime.new(2018, 2, 1), access_token: 'token', token_type: 'Bearer') }
+      let!(:user) { Fabricate(:user, created_at: DateTime.new(2018, 2, 1), access_token: 'token', token_expires_at: Time.now + 1.day, token_type: 'Bearer') }
       it 'retrieves multiple pages of activities', vcr: { cassette_name: 'strava/user_sync_new_strava_activities_many' } do
         expect {
           user.sync_new_strava_activities!
         }.to change(user.activities, :count).by(14)
       end
     end
     context 'different connected_to_strava_at' do
-      let!(:user) { Fabricate(:user, connected_to_strava_at: DateTime.new(2018, 2, 1), access_token: 'token', token_type: 'Bearer') }
+      let!(:user) { Fabricate(:user, connected_to_strava_at: DateTime.new(2018, 2, 1), access_token: 'token', token_expires_at: Time.now + 1.day, token_type: 'Bearer') }
       it 'retrieves multiple pages of activities', vcr: { cassette_name: 'strava/user_sync_new_strava_activities_many' } do
         expect {
           user.sync_new_strava_activities!
         }.to change(user.activities, :count).by(14)
       end
     end
     context 'with private activities', vcr: { cassette_name: 'strava/user_sync_new_strava_activities_with_private' } do
-      let!(:user) { Fabricate(:user, created_at: DateTime.new(2018, 3, 26), access_token: 'token', token_type: 'Bearer') }
+      let!(:user) { Fabricate(:user, created_at: DateTime.new(2018, 3, 26), access_token: 'token', token_expires_at: Time.now + 1.day, token_type: 'Bearer') }
       context 'by default' do
         it 'skips private activities' do
           expect {
@@ -176,7 +200,7 @@
   end
   context '#dm_connect!' do
     let(:user) { Fabricate(:user) }
-    let(:url) { "https://www.strava.com/oauth/authorize?client_id=&redirect_uri=https://slava.playplay.io/connect&response_type=code&scope=view_private&state=#{user.id}" }
+    let(:url) { "https://www.strava.com/oauth/authorize?client_id=&redirect_uri=https://slava.playplay.io/connect&response_type=code&scope=activity:read_all&state=#{user.id}" }
     it 'uses the default message' do
       expect(user).to receive(:dm!).with(
         text: 'Please connect your Strava account.',

spec/slack-strava/commands/connect_spec.rb

@@ -12,7 +12,7 @@
     context 'subscribed team' do
       let(:team) { Fabricate(:team, subscribed: true) }
       let(:user) { Fabricate(:user, team: team) }
-      let(:url) { "https://www.strava.com/oauth/authorize?client_id=&redirect_uri=https://slava.playplay.io/connect&response_type=code&scope=view_private&state=#{user.id}" }
+      let(:url) { "https://www.strava.com/oauth/authorize?client_id=&redirect_uri=https://slava.playplay.io/connect&response_type=code&scope=activity:read_all&state=#{user.id}" }
       it 'connects a user', vcr: { cassette_name: 'slack/user_info' } do
         expect(User).to receive(:find_create_or_update_by_slack_id!).and_return(user)
         expect(user).to receive(:dm!).with(

spec/slack-strava/server_spec.rb

@@ -14,7 +14,7 @@
   end
   context '#member_joined_channel' do
     let(:user) { Fabricate(:user, team: team) }
-    let(:connect_url) { "https://www.strava.com/oauth/authorize?client_id=&redirect_uri=https://slava.playplay.io/connect&response_type=code&scope=view_private&state=#{user.id}" }
+    let(:connect_url) { "https://www.strava.com/oauth/authorize?client_id=&redirect_uri=https://slava.playplay.io/connect&response_type=code&scope=activity:read_all&state=#{user.id}" }
     it 'offers to connect account', vcr: { cassette_name: 'slack/user_info' } do
       allow(client).to receive(:self).and_return(Hashie::Mash.new(id: 'U12345'))
       allow(User).to receive(:find_create_or_update_by_slack_id!).and_return(user)