Skip to content
Snippets Groups Projects
Commit f17b7750 authored by Drew Blessing's avatar Drew Blessing :three: Committed by Adam Hegyi
Browse files

Query only distinct OAuth access tokens by application ID

With recent changes all access tokens for a given application were
being displayed rather than just the latest one, as previously.
With this change only the latest token will be displayed.

Changelog: fixed
parent b1370b1d
No related branches found
No related tags found
1 merge request!91310Query only distinct OAuth access tokens by application ID
......@@ -53,7 +53,9 @@ def verify_user_oauth_applications_enabled
def set_index_vars
@applications = current_user.oauth_applications.load
@authorized_tokens = current_user.oauth_authorized_tokens.preload(:application).order(created_at: :desc).load # rubocop: disable CodeReuse/ActiveRecord
@authorized_tokens = current_user.oauth_authorized_tokens
.latest_per_application
.preload_application
# Don't overwrite a value possibly set by `create`
@application ||= Doorkeeper::Application.new
......
......@@ -7,6 +7,8 @@ class OauthAccessToken < Doorkeeper::AccessToken
alias_attribute :user, :resource_owner
scope :distinct_resource_owner_counts, ->(applications) { where(application: applications).distinct.group(:application_id).count(:resource_owner_id) }
scope :latest_per_application, -> { select('distinct on(application_id) *').order(application_id: :desc, created_at: :desc) }
scope :preload_application, -> { preload(:application) }
def scopes=(value)
if value.is_a?(Array)
......
......@@ -1625,7 +1625,7 @@ def namespaces(owned_only: false)
end
def oauth_authorized_tokens
Doorkeeper::AccessToken.where(resource_owner_id: id, revoked_at: nil)
OauthAccessToken.where(resource_owner_id: id, revoked_at: nil)
end
# Returns the projects a user contributed to in the last year.
......
# frozen_string_literal: true
class AddPartialIndexOnOauthAccessTokensRevokedAtWithOrder < Gitlab::Database::Migration[2.0]
disable_ddl_transaction!
INDEX_NAME = 'partial_index_user_id_app_id_created_at_token_not_revoked'
EXISTING_INDEX_NAME = 'partial_index_resource_owner_id_created_at_token_not_revoked'
def up
add_concurrent_index :oauth_access_tokens, [:resource_owner_id, :application_id, :created_at],
name: INDEX_NAME, where: 'revoked_at IS NULL'
remove_concurrent_index :oauth_access_tokens, [:resource_owner_id, :created_at], name: EXISTING_INDEX_NAME
end
def down
add_concurrent_index :oauth_access_tokens, [:resource_owner_id, :created_at],
name: EXISTING_INDEX_NAME, where: 'revoked_at IS NULL'
remove_concurrent_index :oauth_access_tokens, [:resource_owner_id, :application_id, :created_at], name: INDEX_NAME
end
end
5b12e0fbebef2979cfc31aab16ce78988a2f94662dbe1048791413347edb3c99
\ No newline at end of file
......@@ -30172,14 +30172,14 @@ CREATE INDEX partial_index_deployments_for_legacy_successful_deployments ON depl
 
CREATE INDEX partial_index_deployments_for_project_id_and_tag ON deployments USING btree (project_id) WHERE (tag IS TRUE);
 
CREATE INDEX partial_index_resource_owner_id_created_at_token_not_revoked ON oauth_access_tokens USING btree (resource_owner_id, created_at) WHERE (revoked_at IS NULL);
CREATE INDEX partial_index_slack_integrations_with_bot_user_id ON slack_integrations USING btree (id) WHERE (bot_user_id IS NOT NULL);
 
CREATE UNIQUE INDEX partial_index_sop_configs_on_namespace_id ON security_orchestration_policy_configurations USING btree (namespace_id) WHERE (namespace_id IS NOT NULL);
 
CREATE UNIQUE INDEX partial_index_sop_configs_on_project_id ON security_orchestration_policy_configurations USING btree (project_id) WHERE (project_id IS NOT NULL);
 
CREATE INDEX partial_index_user_id_app_id_created_at_token_not_revoked ON oauth_access_tokens USING btree (resource_owner_id, application_id, created_at) WHERE (revoked_at IS NULL);
CREATE UNIQUE INDEX snippet_user_mentions_on_snippet_id_and_note_id_index ON snippet_user_mentions USING btree (snippet_id, note_id);
 
CREATE UNIQUE INDEX snippet_user_mentions_on_snippet_id_index ON snippet_user_mentions USING btree (snippet_id) WHERE (note_id IS NULL);
......@@ -45,10 +45,12 @@
let(:anonymous_token) { create(:oauth_access_token, resource_owner: user) }
context 'with multiple access token types and multiple owners' do
let!(:token2) { create(:oauth_access_token, application: application, resource_owner: user) }
let!(:other_user_token) { create(:oauth_access_token, application: application, resource_owner: other_user) }
before do
token.update_column(:created_at, created_at)
token2.update_column(:created_at, created_at - 1.day)
anonymous_token.update_columns(application_id: nil, created_at: 1.day.ago)
end
......
......@@ -7,22 +7,40 @@
let(:app_one) { create(:oauth_application) }
let(:app_two) { create(:oauth_application) }
let(:app_three) { create(:oauth_application) }
let(:tokens) { described_class.all }
let(:token) { create(:oauth_access_token, application_id: app_one.id) }
before do
create(:oauth_access_token, application_id: app_one.id)
create_list(:oauth_access_token, 2, resource_owner: user, application_id: app_two.id)
end
describe 'scopes' do
describe '.distinct_resource_owner_counts' do
let(:tokens) { described_class.all }
before do
token
create_list(:oauth_access_token, 2, resource_owner: user, application_id: app_two.id)
end
it 'returns unique owners' do
expect(tokens.count).to eq(3)
expect(tokens.distinct_resource_owner_counts([app_one])).to eq({ app_one.id => 1 })
expect(tokens.distinct_resource_owner_counts([app_two])).to eq({ app_two.id => 1 })
expect(tokens.distinct_resource_owner_counts([app_three])).to eq({})
expect(tokens.distinct_resource_owner_counts([app_one, app_two]))
.to eq({
app_one.id => 1,
app_two.id => 1
})
end
end
describe '.latest_per_application' do
let!(:app_two_token1) { create(:oauth_access_token, application: app_two) }
let!(:app_two_token2) { create(:oauth_access_token, application: app_two) }
let!(:app_three_token1) { create(:oauth_access_token, application: app_three) }
let!(:app_three_token2) { create(:oauth_access_token, application: app_three) }
it 'returns unique owners' do
expect(tokens.count).to eq(3)
expect(tokens.distinct_resource_owner_counts([app_one])).to eq({ app_one.id => 1 })
expect(tokens.distinct_resource_owner_counts([app_two])).to eq({ app_two.id => 1 })
expect(tokens.distinct_resource_owner_counts([app_three])).to eq({})
expect(tokens.distinct_resource_owner_counts([app_one, app_two]))
.to eq({
app_one.id => 1,
app_two.id => 1
})
it 'returns only the latest token for each application' do
expect(described_class.latest_per_application.map(&:id))
.to match_array([app_two_token2.id, app_three_token2.id])
end
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment