Skip to content

Commit

Permalink
Revoke access tokens when user is being destroyed or disconnected.
Browse files Browse the repository at this point in the history
  • Loading branch information
dblock committed Feb 20, 2024
1 parent 1ac51fb commit 2f92839
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 4 deletions.
4 changes: 2 additions & 2 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2024-02-18 03:42:33 UTC using RuboCop version 1.31.2.
# on 2024-02-20 14:59:03 UTC using RuboCop version 1.31.2.
# 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
Expand Down Expand Up @@ -184,7 +184,7 @@ Style/StringConcatenation:
- 'slack-strava/models/team.rb'
- 'tasks/db.rake'

# Offense count: 162
# Offense count: 163
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns, IgnoredPatterns.
# URISchemes: http, https
Expand Down
12 changes: 11 additions & 1 deletion slack-strava/models/strava_tokens.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,20 @@ def refresh_access_token!
)
end

def revoke_access_token!
return unless access_token

response = strava_client.deauthorize(
access_token: access_token
)

raise 'Missing access_token in deauthorize response.' unless response.access_token
end

def strava_client
@strava_client ||= begin
refresh_access_token!
raise 'Missing access_token' unless access_token
raise 'Missing access_token.' unless access_token

Strava::Api::Client.new(access_token: access_token)
end
Expand Down
12 changes: 11 additions & 1 deletion slack-strava/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,9 @@ def connect!(code)

def disconnect_from_strava
if access_token
logger.info "Disconnected team=#{team_id}, user=#{user_name}, user_id=#{id}"
try_to_revoke_access_token
reset_access_tokens!(connected_to_strava_at: nil)
logger.info "Disconnected team=#{team_id}, user=#{user_name}, user_id=#{id}"
{ text: 'Your Strava account has been successfully disconnected.' }
else
{ text: 'Your Strava account is not connected.' }
Expand Down Expand Up @@ -296,8 +297,17 @@ def team_admin?
activated_user? || is_admin? || is_owner?
end

before_destroy :try_to_revoke_access_token

private

def try_to_revoke_access_token
revoke_access_token!
logger.info "Revoked access token for team=#{team_id}, user=#{user_name}, user_id=#{id}"
rescue StandardError => e
logger.warn "Error revoking access token for #{self}: #{e.message}"
end

# includes some of the most recent activities
def before_connected_to_strava_at(tt = 8.hours)
dt = connected_to_strava_at
Expand Down
11 changes: 11 additions & 0 deletions spec/models/team_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,15 @@
Timecop.return
end
end
context '#destroy' do
let!(:team) { Fabricate(:team) }
let!(:user1) { Fabricate(:user, team: team) }
let!(:user2) { Fabricate(:user, team: team, access_token: 'token', token_expires_at: Time.now + 1.day, token_type: 'Bearer') }
it 'revokes access tokens' do
allow(team).to receive(:users).and_return([user1, user2])
expect(user1).to receive(:revoke_access_token!)
expect(user2).to receive(:revoke_access_token!)
team.destroy
end
end
end
18 changes: 18 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -488,4 +488,22 @@
end
end
end
context '#destroy' do
context 'without an access token' do
let!(:user) { Fabricate(:user) }
it 'revokes access token' do
expect_any_instance_of(Strava::Api::Client).to_not receive(:deauthorize)
user.destroy
end
end
context 'with an access token' do
let!(:user) { Fabricate(:user, access_token: 'token', token_expires_at: Time.now + 1.day, token_type: 'Bearer') }
it 'revokes access token' do
expect(user.strava_client).to receive(:deauthorize)
.with(access_token: user.access_token)
.and_return(access_token: user.access_token)
user.destroy
end
end
end
end

0 comments on commit 2f92839

Please sign in to comment.