diff --git a/app/controllers/admin/api/access_tokens_controller.rb b/app/controllers/admin/api/access_tokens_controller.rb index 5665190e92..4372085b0e 100644 --- a/app/controllers/admin/api/access_tokens_controller.rb +++ b/app/controllers/admin/api/access_tokens_controller.rb @@ -22,7 +22,7 @@ def create protected def access_token_params - params.require(:token).permit(:name, :permission, scopes: []) + params.require(:token).permit(:name, :permission, :expires_at, scopes: []) end def authorize_access_tokens diff --git a/app/controllers/admin/api/personal/access_tokens_controller.rb b/app/controllers/admin/api/personal/access_tokens_controller.rb index f49636159e..38c9dcb714 100644 --- a/app/controllers/admin/api/personal/access_tokens_controller.rb +++ b/app/controllers/admin/api/personal/access_tokens_controller.rb @@ -39,6 +39,6 @@ def token end def access_token_params - params.require(:token).permit(:name, :permission, scopes: []) + params.require(:token).permit(:name, :permission, :expires_at, scopes: []) end end diff --git a/app/lib/api_authentication/by_access_token.rb b/app/lib/api_authentication/by_access_token.rb index 774571f21f..cc114011b4 100644 --- a/app/lib/api_authentication/by_access_token.rb +++ b/app/lib/api_authentication/by_access_token.rb @@ -105,7 +105,15 @@ def show_access_key_permission_error def authenticated_token return @authenticated_token if instance_variable_defined?(:@authenticated_token) - @authenticated_token = domain_account.access_tokens.find_from_value(access_token) if access_token + given_token = access_token + + return if given_token.blank? + + token = domain_account.access_tokens.find_from_value(given_token) + + return if token.blank? || token.expired? + + @authenticated_token = token end def enforce_access_token_permission(&block) diff --git a/app/models/access_token.rb b/app/models/access_token.rb index 10db8f52bf..1b1b79cad6 100644 --- a/app/models/access_token.rb +++ b/app/models/access_token.rb @@ -1,4 +1,8 @@ class AccessToken < ApplicationRecord + TIMESTAMP_FORMAT = '%FT%T%:z'.freeze + PAST_TIME = Time.at(0).utc.freeze + private_constant :PAST_TIME + belongs_to :owner, class_name: 'User', inverse_of: :access_tokens validates :name, length: { maximum: 255 } @@ -92,10 +96,11 @@ def self.allowed_scopes validates :permission, inclusion: { in: PERMISSIONS.values }, length: { maximum: 255 } validates :scopes, length: { minimum: 1, maximum: 65535 } validate :validate_scope_exists + validate :validate_expiration_date, on: %i[create] after_initialize :generate_value - attr_accessible :owner, :name, :scopes, :permission + attr_accessible :owner, :name, :scopes, :permission, :expires_at attr_readonly :value @@ -132,6 +137,24 @@ def validate_scope_exists errors.add :scopes, :invalid end + def expires_at=(value) + return if value.blank? + + DateTime.strptime(value) + + super value + rescue StandardError + super PAST_TIME + end + + def validate_expiration_date + return true if expires_at.blank? + + return true if expires_at > Time.now.utc + + errors.add :expires_at, :invalid, message: "Date must follow ISO8601 format and be future. Example: #{1.week.from_now.utc.iso8601}" + end + def generate_value self.value ||= self.class.random_id end @@ -159,4 +182,8 @@ def human_scopes def self.random_id SecureRandom.hex(32) end + + def expired? + expires_at.present? && expires_at < Time.now.utc + end end diff --git a/app/representers/access_token_representer.rb b/app/representers/access_token_representer.rb index d17799b89a..c763709319 100644 --- a/app/representers/access_token_representer.rb +++ b/app/representers/access_token_representer.rb @@ -11,5 +11,6 @@ class AccessTokenRepresenter < ThreeScale::Representer property :name property :scopes property :permission + property :expires_at property :value, if: :show_value? end diff --git a/config/routes.rb b/config/routes.rb index 05316b41c2..4631ed3176 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -516,7 +516,7 @@ get 'objects/status' => 'objects#status', as: :objects_status, controller: :objects, defaults: { format: :json } namespace :personal, defaults: { format: :json } do - resources :access_tokens, except: %i[new edit] + resources :access_tokens, except: %i[new edit update] end # /admin/api/provider diff --git a/doc/active_docs/account_management_api.json b/doc/active_docs/account_management_api.json index 5cd39d95a3..ae3a331e8b 100644 --- a/doc/active_docs/account_management_api.json +++ b/doc/active_docs/account_management_api.json @@ -3006,6 +3006,12 @@ "policy_registry" ] } + }, + "expires_at": { + "type": "string", + "format": "date-time", + "description": "Expiration time of the access token. In ISO 8601 format", + "example": "2030-01-01T12:00:00Z" } }, "required": [ @@ -7149,6 +7155,12 @@ "policy_registry" ] } + }, + "expires_at": { + "type": "string", + "format": "date-time", + "description": "Expiration time of the access token. In ISO 8601 format", + "example": "2030-01-01T12:00:00Z" } }, "required": [ diff --git a/test/integration/api/access_tokens_test.rb b/test/integration/api/access_tokens_test.rb index 3d20fc84d5..cfd026268f 100644 --- a/test/integration/api/access_tokens_test.rb +++ b/test/integration/api/access_tokens_test.rb @@ -50,6 +50,18 @@ def setup end end + test 'create accepts an expiration time' do + access_token = FactoryBot.create(:access_token, owner: @admin, scopes: %w[account_management]) + + user_id = @admin.id + expires_at = 1.day.from_now.utc.iso8601 + assert_difference(AccessToken.method(:count), 1) do + post_request(user_id, {access_token: access_token.value}, { expires_at: }) + assert_response :created, "Not created with response body #{response.body}" + end + assert_equal expires_at, AccessToken.last!.expires_at.iso8601 + end + test 'create with provider_key can create for any user of that account' do FactoryBot.create(:cinstance, service: master_account.default_service, user_account: @provider) diff --git a/test/integration/api/personal/access_tokens_test.rb b/test/integration/api/personal/access_tokens_test.rb index f8539f87f9..9729d5beea 100644 --- a/test/integration/api/personal/access_tokens_test.rb +++ b/test/integration/api/personal/access_tokens_test.rb @@ -95,6 +95,15 @@ class Admin::Api::Personal::CreateAccessTokenTest < Admin::Api::Personal::Access end end + test 'POST accepts an expiration time' do + expires_at = 1.day.from_now.utc.iso8601 + assert_difference @admin.access_tokens.method(:count) do + create_access_token(access_token: admin_access_token.value, params: access_token_params({ expires_at: })) + assert_response :created + assert_equal expires_at, JSON.parse(response.body).dig('access_token', 'expires_at') + end + end + def assert_it_worked(_access_token = nil) assert_response :created created_token = AccessToken.last diff --git a/test/integration/by_access_token_integration_test.rb b/test/integration/by_access_token_integration_test.rb index e19a3d31fb..ae864868bf 100644 --- a/test/integration/by_access_token_integration_test.rb +++ b/test/integration/by_access_token_integration_test.rb @@ -59,4 +59,27 @@ def test_index_with_access_token get admin_api_registry_policies_path(format: :json), headers: auth_headers assert_response :forbidden end + + test 'the token has no expiration date' do + get admin_api_accounts_path(format: :xml), params: { access_token: @token.value } + + assert_response :success + end + + test 'the token has a future expiration date' do + token = FactoryBot.create(:access_token, owner: @user, scopes: 'account_management', expires_at: 1.day.from_now.utc.iso8601) + + get admin_api_accounts_path(format: :xml), params: { access_token: token.value } + + assert_response :success + end + + test 'the token has a past expiration date' do + token = FactoryBot.create(:access_token, owner: @user, scopes: 'account_management') + token.update_columns(expires_at: 1.minute.ago) + + get admin_api_accounts_path(format: :xml), params: { access_token: token.value } + + assert_response :forbidden + end end diff --git a/test/models/access_token_test.rb b/test/models/access_token_test.rb index 2beccbc42b..3e65b49806 100644 --- a/test/models/access_token_test.rb +++ b/test/models/access_token_test.rb @@ -175,6 +175,42 @@ def test_find_from_id_or_value_and_bang assert_equal expected_audited_changes, audit.audited_changes end + test 'expiration time is not mandatory' do + access_token = FactoryBot.build(:access_token) + + assert access_token.valid? + end + + test "expiration time can be blank" do + access_token = FactoryBot.build(:access_token, expires_at: '') + + assert access_token.valid? + end + + test "expiration time can be nil" do + access_token = FactoryBot.build(:access_token, expires_at: nil) + + assert access_token.valid? + end + + test "expiration time can't be invalid" do + access_token = FactoryBot.build(:access_token, expires_at: 'invalid') + + assert_not access_token.valid? + end + + test "expiration time can't be in the past" do + access_token = FactoryBot.build(:access_token, expires_at: 1.day.ago.utc.iso8601) + + assert_not access_token.valid? + end + + test "expiration time accepts a valid ISO 8601 datetime" do + access_token = FactoryBot.build(:access_token, expires_at: 1.year.from_now.utc.iso8601) + + assert access_token.valid? + end + private def assert_access_token_audit_all_data(access_token, audit) diff --git a/test/unit/api_authentication/by_authentication_token_test.rb b/test/unit/api_authentication/by_authentication_token_test.rb index 1705bea495..d81cf92839 100644 --- a/test/unit/api_authentication/by_authentication_token_test.rb +++ b/test/unit/api_authentication/by_authentication_token_test.rb @@ -22,7 +22,7 @@ def setup def mock_token(attributes = {}) @params = { access_token: 'some-token' } - token = mock('access-token', attributes) + token = mock('access-token', attributes.merge(expired?: false)) @access_tokens.expects(:find_from_value).with('some-token').returns(token) token end