From 16c1e5bd404401bcb96cddc0f0cd0ceb1c7ddd87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= <esanz-garcia@gitlab.com>
Date: Tue, 4 Mar 2025 19:44:52 +0100
Subject: [PATCH] Add optional parameters to access tokens

---
 .../helpers/personal_access_tokens_helpers.rb |   4 +
 lib/api/resource_access_tokens.rb             |   7 +-
 .../api/personal_access_tokens_spec.rb        |  31 ++++
 .../api/resource_access_tokens_spec.rb        | 141 +++++++++++++++---
 .../lib/api/access_token_shared_examples.rb   |  42 ++++--
 5 files changed, 184 insertions(+), 41 deletions(-)

diff --git a/lib/api/helpers/personal_access_tokens_helpers.rb b/lib/api/helpers/personal_access_tokens_helpers.rb
index b1057987604c8..cab5ec02e1f50 100644
--- a/lib/api/helpers/personal_access_tokens_helpers.rb
+++ b/lib/api/helpers/personal_access_tokens_helpers.rb
@@ -18,6 +18,10 @@ module PersonalAccessTokensHelpers
           documentation: { example: '2021-01-01' }
         optional :last_used_after, type: DateTime, desc: 'Filter tokens which were used after given datetime',
           documentation: { example: '2022-01-01' }
+        optional :expires_before, type: Date, desc: 'Filter tokens which expire before given datetime',
+          documentation: { example: '2022-01-01' }
+        optional :expires_after, type: Date, desc: 'Filter tokens which expire after given datetime',
+          documentation: { example: '2021-01-01' }
         optional :search, type: String, desc: 'Filters tokens by name', documentation: { example: 'token' }
         optional :sort, type: String, desc: 'Sort tokens', documentation: { example: 'created_at_desc' }
       end
diff --git a/lib/api/resource_access_tokens.rb b/lib/api/resource_access_tokens.rb
index 30b5d18398919..161a68a1bcc09 100644
--- a/lib/api/resource_access_tokens.rb
+++ b/lib/api/resource_access_tokens.rb
@@ -10,6 +10,8 @@ class ResourceAccessTokens < ::API::Base
 
     feature_category :system_access
 
+    helpers ::API::Helpers::PersonalAccessTokensHelpers
+
     %w[project group].each do |source_type|
       resource source_type.pluralize, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
         desc 'Get list of all access tokens for the specified resource' do
@@ -20,15 +22,14 @@ class ResourceAccessTokens < ::API::Base
         end
         params do
           requires :id, types: [String, Integer], desc: "ID or URL-encoded path of the #{source_type}"
-          optional :state, type: String, desc: 'Filter tokens which are either active or inactive',
-            values: %w[active inactive], documentation: { example: 'active' }
+          use :access_token_params
         end
         get ":id/access_tokens" do
           resource = find_source(source_type, params[:id])
 
           next unauthorized! unless current_user.can?(:read_resource_access_tokens, resource)
 
-          tokens = PersonalAccessTokensFinder.new({ user: resource.bots, impersonation: false, state: params[:state] }).execute.preload_users
+          tokens = PersonalAccessTokensFinder.new(declared(params, include_missing: false).merge({ user: resource.bots, impersonation: false })).execute.preload_users
 
           resource.members.load
           present paginate(tokens), with: Entities::ResourceAccessToken, resource: resource
diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb
index 674415f0b20d0..f84bf96c50cc5 100644
--- a/spec/requests/api/personal_access_tokens_spec.rb
+++ b/spec/requests/api/personal_access_tokens_spec.rb
@@ -180,6 +180,37 @@ def map_id(json_resonse)
         end
       end
 
+      context 'filter with expires parameter' do
+        let_it_be(:token1) { create(:personal_access_token, expires_at: Date.new(2022, 01, 01)) }
+
+        context 'test expires_before' do
+          where(:expires_at, :status, :result_count, :result) do
+            '2022-01-02'          | :ok          | 1 | lazy { [token1.id] }
+            '2022-01-01'          | :ok          | 0 | lazy { [] }
+            '2022-01-01T12:30:24' | :ok          | 0 | lazy { [] }
+            'asdf'                | :bad_request | 1 | { "error" => "expires_before is invalid" }
+          end
+
+          with_them do
+            it_behaves_like 'response as expected', expires_before: params[:expires_at]
+          end
+        end
+
+        context 'test expires_after' do
+          where(:expires_at, :status, :result_count, :result) do
+            '2022-01-03'            | :ok          | 1 | lazy { [current_users_token.id] }
+            '2022-01-01'            | :ok          | 2 | lazy { [token1.id, current_users_token.id] }
+            '2022-01-01T12:30:26'   | :ok          | 2 | lazy { [token1.id, current_users_token.id] }
+            (DateTime.now + 1).to_s | :ok          | 1 | lazy { [current_users_token.id] }
+            'asdf'                  | :bad_request | 1 | { "error" => "expires_after is invalid" }
+          end
+
+          with_them do
+            it_behaves_like 'response as expected', expires_after: params[:expires_at]
+          end
+        end
+      end
+
       context 'filter with search parameter' do
         let_it_be(:token1) { create(:personal_access_token, name: 'test_1') }
         let_it_be(:token2) { create(:personal_access_token, name: 'test_2') }
diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb
index dd0d5fe61c9cf..01f2237c3f0f0 100644
--- a/spec/requests/api/resource_access_tokens_spec.rb
+++ b/spec/requests/api/resource_access_tokens_spec.rb
@@ -4,7 +4,7 @@
 
 RSpec.describe API::ResourceAccessTokens, feature_category: :system_access do
   let_it_be(:user) { create(:user) }
-  let_it_be(:user_non_priviledged) { create(:user) }
+  let_it_be(:user_non_privileged) { create(:user) }
 
   shared_examples 'resource access token API' do |source_type|
     context "GET #{source_type}s/:id/access_tokens" do
@@ -13,7 +13,11 @@
       context "when the user has valid permissions" do
         let_it_be(:project_bot) { create(:user, :project_bot, bot_namespace: namespace) }
         let_it_be(:active_access_tokens) { create_list(:personal_access_token, 5, user: project_bot) }
-        let_it_be(:expired_token) { create(:personal_access_token, :expired, user: project_bot) }
+        let_it_be(:expired_token) do
+          create(:personal_access_token, :expired, expires_at: 2.days.ago, last_used_at: 2.days.ago, name: 'a_test_1',
+            user: project_bot)
+        end
+
         let_it_be(:revoked_token) { create(:personal_access_token, :revoked, user: project_bot) }
         let_it_be(:inactive_access_tokens) { [expired_token, revoked_token] }
         let_it_be(:all_access_tokens) { active_access_tokens + inactive_access_tokens }
@@ -122,39 +126,128 @@
           end
         end
 
-        context 'when state param is set to inactive' do
-          let(:params) { { state: 'inactive' } }
+        context 'when filtering by revoked' do
+          it 'returns non-revoked tokens when revoked is false' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { revoked: false }
 
-          it 'returns only inactive tokens' do
-            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: params
+            token_ids = json_response.map { |token| token['id'] }
+            expect(token_ids).to match_array(all_access_tokens.pluck(:id).reject { |n| n == revoked_token.id })
+          end
+
+          it 'returns revoked tokens when revoked is true' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { revoked: true }
 
             token_ids = json_response.map { |token| token['id'] }
+            expect(token_ids).to match_array([revoked_token.id])
+          end
+        end
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(response).to include_pagination_headers
-            expect(response).to match_response_schema('public_api/v4/resource_access_tokens')
-            expect(token_ids).to match_array(inactive_access_tokens.pluck(:id))
+        context 'when filtering by state' do
+          context 'when state param is set to inactive' do
+            let(:params) { { state: 'inactive' } }
+
+            it 'returns only inactive tokens' do
+              get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: params
+
+              token_ids = json_response.map { |token| token['id'] }
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(response).to match_response_schema('public_api/v4/resource_access_tokens')
+              expect(token_ids).to match_array(inactive_access_tokens.pluck(:id))
+            end
+          end
+
+          context 'when state param is set to active' do
+            let(:params) { { state: 'active' } }
+
+            it 'returns only active tokens' do
+              get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: params
+
+              token_ids = json_response.map { |token| token['id'] }
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(response).to match_response_schema('public_api/v4/resource_access_tokens')
+              expect(token_ids).to match_array(active_access_tokens.pluck(:id))
+            end
           end
         end
 
-        context 'when state param is set to active' do
-          let(:params) { { state: 'active' } }
+        context 'when filtering by created dates' do
+          it 'returns tokens created before specified date' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { created_before: 1.day.ago }
 
-          it 'returns only active tokens' do
-            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: params
+            expect(json_response).to be_empty
+          end
+
+          it 'returns tokens created after specified date' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { created_after: 1.day.ago }
 
             token_ids = json_response.map { |token| token['id'] }
+            expect(token_ids).to match_array(all_access_tokens.pluck(:id))
+          end
+        end
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(response).to include_pagination_headers
-            expect(response).to match_response_schema('public_api/v4/resource_access_tokens')
-            expect(token_ids).to match_array(active_access_tokens.pluck(:id))
+        context 'when filtering by last used dates' do
+          it 'returns tokens last used before specified date' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { last_used_before: 1.day.ago }
+
+            token_ids = json_response.map { |token| token['id'] }
+            expect(token_ids).to match_array([expired_token.id])
+          end
+
+          it 'returns tokens last used after specified date' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { last_used_after: 1.day.ago }
+
+            expect(json_response).to be_empty
+          end
+        end
+
+        context 'when filtering by expiration dates' do
+          it 'returns tokens that expire before specified date' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { expires_before: 1.day.ago }
+
+            token_ids = json_response.map { |token| token['id'] }
+            expect(token_ids).to match_array([expired_token.id])
+          end
+
+          it 'returns tokens that expire after specified date' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { expires_after: 1.day.ago }
+
+            token_ids = json_response.map { |token| token['id'] }
+            expect(token_ids).to match_array(all_access_tokens.pluck(:id).reject { |n| n == expired_token.id })
+          end
+        end
+
+        context 'when searching by name' do
+          it 'returns tokens matching the search term' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { search: 'a_test_1' }
+
+            token_ids = json_response.map { |token| token['id'] }
+            expect(token_ids).to match_array([expired_token.id])
+          end
+        end
+
+        context 'when sorting' do
+          it 'sorts tokens by last_used_desc when specified' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { sort: 'name_desc' }
+
+            token_ids = json_response.map { |token| token['id'] }
+            expect(token_ids.last).to eq(expired_token.id)
+          end
+
+          it 'sorts tokens by last_used_asc when specified' do
+            get api("/#{source_type}s/#{resource_id}/access_tokens", user), params: { sort: 'name_asc' }
+
+            token_ids = json_response.map { |token| token['id'] }
+            expect(token_ids.first).to eq(expired_token.id)
           end
         end
       end
 
       context "when the user does not have valid permissions" do
-        let_it_be(:user) { user_non_priviledged }
+        let_it_be(:user) { user_non_privileged }
         let_it_be(:project_bot) { create(:user, :project_bot, bot_namespace: namespace) }
         let_it_be(:access_tokens) { create_list(:personal_access_token, 3, user: project_bot) }
         let_it_be(:resource_id) { resource.id }
@@ -266,7 +359,7 @@
       end
 
       context "when the user does not have valid permissions" do
-        let_it_be(:user) { user_non_priviledged }
+        let_it_be(:user) { user_non_privileged }
 
         it "returns 401" do
           get_token
@@ -340,7 +433,7 @@
       end
 
       context "when the user does not have valid permissions" do
-        let_it_be(:user) { user_non_priviledged }
+        let_it_be(:user) { user_non_privileged }
 
         it "does not delete the token, and returns 400", :aggregate_failures do
           delete_token
@@ -483,7 +576,7 @@
         let_it_be(:resource_id) { resource.id }
 
         context "when the user role is too low" do
-          let_it_be(:user) { user_non_priviledged }
+          let_it_be(:user) { user_non_privileged }
 
           it "does not create the token, and returns the permission error" do
             create_token
@@ -687,7 +780,7 @@
     before_all do
       resource.add_maintainer(user)
       other_resource.add_maintainer(user)
-      resource.add_developer(user_non_priviledged)
+      resource.add_developer(user_non_privileged)
     end
 
     it_behaves_like 'resource access token API', 'project'
@@ -703,7 +796,7 @@
     before_all do
       resource.add_owner(user)
       other_resource.add_owner(user)
-      resource.add_maintainer(user_non_priviledged)
+      resource.add_maintainer(user_non_privileged)
     end
 
     it_behaves_like 'resource access token API', 'group'
diff --git a/spec/support/shared_examples/lib/api/access_token_shared_examples.rb b/spec/support/shared_examples/lib/api/access_token_shared_examples.rb
index daa389ee2593e..801eaa70dc926 100644
--- a/spec/support/shared_examples/lib/api/access_token_shared_examples.rb
+++ b/spec/support/shared_examples/lib/api/access_token_shared_examples.rb
@@ -43,6 +43,20 @@
     )
   end
 
+  context 'when filtering by revoked' do
+    it 'returns not-revoked tokens when revoked is false' do
+      get api_request, params: { revoked: false }
+
+      expect_paginated_array_response_contain_exactly(*all_token_ids.excluding(revoked_token1.id, revoked_token2.id))
+    end
+
+    it 'returns revoked tokens when revoked is true' do
+      get api_request, params: { revoked: true }
+
+      expect_paginated_array_response_contain_exactly(revoked_token1.id, revoked_token2.id)
+    end
+  end
+
   context 'when filtering by state' do
     it 'returns active tokens when state is active' do
       get api_request, params: { state: 'active' }
@@ -68,20 +82,6 @@
     end
   end
 
-  context 'when filtering by revoked' do
-    it 'returns not-revoked tokens when revoked is false' do
-      get api_request, params: { revoked: false }
-
-      expect_paginated_array_response_contain_exactly(*all_token_ids.excluding(revoked_token1.id, revoked_token2.id))
-    end
-
-    it 'returns revoked tokens when revoked is true' do
-      get api_request, params: { revoked: true }
-
-      expect_paginated_array_response_contain_exactly(revoked_token1.id, revoked_token2.id)
-    end
-  end
-
   context 'when filtering by created dates' do
     it 'returns tokens created before specified date' do
       get api_request, params: { created_before: 1.day.ago }
@@ -110,6 +110,20 @@
     end
   end
 
+  context 'when filtering by expiration dates' do
+    it 'returns tokens that expire before specified date' do
+      get api_request, params: { expires_before: 1.year.ago + 1.day }
+
+      expect_paginated_array_response_contain_exactly(expired_token1.id, expired_token2.id)
+    end
+
+    it 'returns tokens that expire after specified date' do
+      get api_request, params: { expires_after: 1.year.ago, expires_before: 1.week.ago }
+
+      expect_paginated_array_response_contain_exactly(expired_token1.id, expired_token2.id)
+    end
+  end
+
   context 'when searching by name' do
     it 'returns tokens matching the search term' do
       get api_request, params: { search: 'test' }
-- 
GitLab