Merge pull request #3939 from 3scale/THREESCALE-10245-expire-access-t…

…okens

THREESCALE-10245: Expirable access tokens

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

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

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)

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

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

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

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": [

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)
 

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

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

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)

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