Skip to content

Commit

Permalink
Revoke doorkeeper token on signout (#4134)
Browse files Browse the repository at this point in the history
* update sessions toke with revoke access tokens on sign out

* add revoke access token specs

* update sessions_controller to revoke all tokens for user for the relevant oauth app. (This is assuming we send over the doorkeeper token on destroy session request)

* update session controller spec to include bearer token header

* update revoke token to send user not user id

* add specs for revoke token on signout

* add sign out spec when bearer token is not supplied

* hound sniff on indentation in sessions_controller_spec and remove accidentally commited logs

* Update app/controllers/sessions_controller.rb

Co-authored-by: Campbell Allen <[email protected]>

* update session controller to remove redundant safe guarding

* update per hound sniff. add line after safeguard clause

---------

Co-authored-by: Campbell Allen <[email protected]>
  • Loading branch information
yuenmichelle1 and camallen authored Mar 9, 2023
1 parent 8e0b9a4 commit fcc1440
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 0 deletions.
13 changes: 13 additions & 0 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def create_from_json
end

def destroy_from_json
revoke_all_user_application_access_tokens! if doorkeeper_token
Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name)
yield if block_given?
head :no_content
Expand All @@ -52,4 +53,16 @@ def set_csrf_headers
response.headers['X-CSRF-Param'] = request_forgery_protection_token.to_s
response.headers['X-CSRF-Token'] = form_authenticity_token.to_s
end

def revoke_all_user_application_access_tokens!
application_id_to_revoke = doorkeeper_token.application_id
return unless application_id_to_revoke

# revoke all tokens that this user owns for the client application this token is linked to, e.g. PFE, FEM, classrooms etc.
user = User.find(doorkeeper_token.resource_owner_id)
Doorkeeper::AccessToken.revoke_all_for(
application_id_to_revoke,
user
)
end
end
58 changes: 58 additions & 0 deletions spec/controllers/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,64 @@
delete :destroy
end
end

describe 'revoking access tokens' do
let(:oauth_app) { create(:non_confidential_first_party_app, owner: user) }
let(:user_token) { create(:access_token, application_id: oauth_app.id, resource_owner_id: user.id) }
let(:user_other_token) { create(:access_token, application_id: oauth_app.id, resource_owner_id: user.id) }
let(:user_revoked_token) { create(:access_token, application_id: oauth_app.id, resource_owner_id: user.id) }
let(:another_user) { create(:user) }
let(:other_user_token) { create(:access_token, application_id: oauth_app.id, resource_owner_id: another_user.id) }
let(:other_oauth_app) { create(:non_confidential_first_party_app, owner: user) }
let(:other_app_token) { create(:access_token, application_id: other_oauth_app.id, resource_owner_id: user.id) }

before do
user_token
user_other_token
user_revoked_token.revoke
other_user_token
other_app_token
request.env['devise.user'] = user
sign_in user
end

it 'revokes all access tokens for the relevant client app' do
request.env['HTTP_AUTHORIZATION'] = "Bearer #{user_token.token}"
expect {
delete :destroy
}.to change {
Doorkeeper::AccessToken.where(application_id: oauth_app.id, resource_owner_id: user.id, revoked_at: nil).count
}.from(2).to(0)
end

it 'does not revoke access token for another user' do
request.env['HTTP_AUTHORIZATION'] = "Bearer #{user_token.token}"
expect {
delete :destroy
}.not_to change {
Doorkeeper::AccessToken.where(application_id: oauth_app.id, resource_owner_id: another_user.id, revoked_at: nil).count
}
end

it 'does not revoke access tokens for other client apps' do
request.env['HTTP_AUTHORIZATION'] = "Bearer #{user_token.token}"
expect {
delete :destroy
}.not_to change {
Doorkeeper::AccessToken.where(application_id: other_oauth_app.id, resource_owner_id: user.id, revoked_at: nil).count
}
end

context 'when bearer token not supplied' do
it 'does not revoke any access tokens' do
expect {
delete :destroy
}.not_to change {
Doorkeeper::AccessToken.where(application_id: oauth_app.id, resource_owner_id: user.id, revoked_at: nil).count
}
end
end
end
end

describe "#new" do
Expand Down

0 comments on commit fcc1440

Please sign in to comment.