Split access_token / authorization_code tests

Makefile

@@ -254,9 +254,10 @@ SPEC_HELPER_MIN_SPECS = \
 	spec/models/carto/layer_spec.rb \
 	spec/models/carto/mobile_app_presenter_spec.rb \
 	spec/models/carto/notification_spec.rb \
+	spec/models/carto/oauth_access_token_spec.rb \
 	spec/models/carto/oauth_app_spec.rb \
 	spec/models/carto/oauth_app_user_spec.rb \
-	spec/models/carto/oauth_authorization_spec.rb \
+	spec/models/carto/oauth_authorization_code_spec.rb \
 	spec/models/carto/overlay_spec.rb \
 	spec/models/carto/rate_limit_spec.rb \
 	spec/models/carto/received_notification_spec.rb \

app/models/carto/api_key.rb

@@ -54,7 +54,7 @@ module Carto
     self.inheritance_column = :_type
 
     belongs_to :user
-    has_one :oauth_authorization, inverse_of: :api_key, dependent: :restrict_with_exception
+    has_one :oauth_access_token, inverse_of: :api_key, dependent: :restrict_with_exception
 
     before_create :create_token, if: ->(k) { k.needs_setup? && !k.token }
     before_create :create_db_config, if: ->(k) { k.needs_setup? && !(k.db_role && k.db_password) }
@@ -121,7 +121,7 @@ module Carto
     end
 
     def self.build_oauth_key(user: Carto::User.find(scope_attributes['user_id']), name:, grants:)
-      create!(
+      new(
         user: user,
         type: TYPE_OAUTH,
         name: name,

app/models/carto/oauth_access_token.rb

@@ -13,12 +13,12 @@ module Carto
     validates :oauth_app_user, presence: true
     validates :api_key, presence: true
 
-    before_create :build_api_key
+    before_validation :ensure_api_key
 
     private
 
-    def build_api_key
-      self.api_key = oauth_app_user.user.api_keys.build_oauth_key(
+    def ensure_api_key
+      self.api_key ||= oauth_app_user.user.api_keys.build_oauth_key(
         name: "oauth_authorization #{id}",
         grants: [{ type: 'apis', apis: [] }]
       )

app/models/carto/oauth_authorization_code.rb

@@ -3,7 +3,7 @@
 require_dependency 'carto/oauth_provider/errors'
 
 module Carto
-  class OauthAuthorization < ActiveRecord::Base
+  class OauthAuthorizationCode < ActiveRecord::Base
     # Multiple of 3 for pretty base64
     CODE_RANDOM_BYTES = 12
 
@@ -12,20 +12,25 @@ module Carto
     belongs_to :oauth_app_user, inverse_of: :oauth_authorization_codes
 
     validates :oauth_app_user, presence: true
+    validates :code, presence: true
 
-    def self.create_with_code!(redirect_uri)
-      create!(code: SecureRandom.urlsafe_base64(CODE_RANDOM_BYTES), redirect_uri: redirect_uri)
-    end
+    before_validation :ensure_code_generated
 
     def exchange!
+      raise OauthProvider::Errors::InvalidGrant.new if expired?
+
       ActiveRecord::Base.transaction do
-        oauth_app_user.oauth_access_tokens.create!(scopes: scopes)
         destroy!
+        oauth_app_user.oauth_access_tokens.create!(scopes: scopes)
       end
     end
 
     private
 
+    def ensure_code_generated
+      self.code ||= SecureRandom.urlsafe_base64(CODE_RANDOM_BYTES)
+    end
+
     def expired?
       created_at < Time.now - CODE_EXPIRATION_TIME
     end

spec/models/carto/oauth_access_token_spec.rb

@@ -0,0 +1,22 @@
+# encoding: utf-8
+
+require 'spec_helper_min'
+
+module Carto
+  describe OauthAccessToken do
+    describe '#validation' do
+      before(:all) do
+        @user = FactoryGirl.create(:carto_user)
+        @app = FactoryGirl.create(:oauth_app, user: @user)
+        @app_user = OauthAppUser.create!(user: @user, oauth_app: @app)
+      end
+
+      it 'auto generates api_key' do
+        access_token = OauthAccessToken.new(oauth_app_user: @app_user)
+        expect(access_token).to(be_valid)
+        expect(access_token.api_key).to(be)
+        expect(access_token.api_key.type).to(eq('oauth'))
+      end
+    end
+  end
+end

spec/models/carto/oauth_authorization_code_spec.rb

@@ -0,0 +1,62 @@
+# encoding: utf-8
+
+require 'spec_helper_min'
+
+module Carto
+  describe OauthAuthorizationCode do
+    describe '#validation' do
+      before(:all) do
+        @user = FactoryGirl.build(:carto_user)
+        @app = FactoryGirl.build(:oauth_app, user: @user)
+        @app_user = OauthAppUser.new(user: @user, oauth_app: @app)
+      end
+
+      it 'validates without redirect_uri and autogenerates code' do
+        authorization = OauthAuthorizationCode.new(oauth_app_user: @app_user)
+        expect(authorization).to(be_valid)
+        expect(authorization.code).to(be_present)
+      end
+
+      it 'validates with redirect_uri and autogenerates code' do
+        authorization = OauthAuthorizationCode.new(oauth_app_user: @app_user, redirect_uri: ['https://redirect'])
+        expect(authorization).to(be_valid)
+        expect(authorization.code).to(be_present)
+      end
+    end
+
+    describe '#exchange!' do
+      before(:all) do
+        @user = FactoryGirl.create(:carto_user)
+        @app = FactoryGirl.create(:oauth_app, user: @user)
+        @app_user = OauthAppUser.create(user: @user, oauth_app: @app)
+      end
+
+      after(:all) do
+        @app_user.destroy
+        @user.destroy
+        @app.destroy
+      end
+
+      before(:each) do
+        @authorization_code = @app_user.oauth_authorization_codes.create!
+      end
+
+      after(:each) do
+        @authorization_code.destroy
+      end
+
+      it 'fails if the code is expired' do
+        @authorization_code.created_at -= 10.minutes
+        expect { @authorization_code.exchange! }.to(raise_error(OauthProvider::Errors::InvalidGrant))
+        expect(Carto::OauthAuthorizationCode.exists?(@authorization_code.id)).to(be_true)
+      end
+
+      it 'creates a new api key and blanks the code' do
+        access_token = @authorization_code.exchange!
+        expect(Carto::OauthAuthorizationCode.exists?(@authorization_code.id)).to(be_false)
+        expect(access_token.api_key).to(be)
+        expect(access_token.api_key.type).to(eq('oauth'))
+      end
+    end
+  end
+end

spec/models/carto/oauth_authorization_spec.rb

@@ -1,78 +0,0 @@
-# encoding: utf-8
-
-require 'spec_helper_min'
-
-module Carto
-  describe OauthAuthorization do
-    describe '#validation' do
-      before(:all) do
-        @user = FactoryGirl.build(:carto_user)
-        @app = FactoryGirl.build(:oauth_app, user: @user)
-        @api_key = FactoryGirl.build(:master_api_key, user: @user)
-        @app_user = OauthAppUser.new(user: @user, oauth_app: @app)
-      end
-
-      it 'requires code or api_key' do
-        authorization = OauthAuthorization.new
-        expect(authorization).to_not(be_valid)
-        expect(authorization.errors[:api_key]).to(include("must be present if code is missing"))
-
-        authorization.code = ''
-        expect(authorization).to_not(be_valid)
-        expect(authorization.errors[:api_key]).to(include("must be present if code is missing"))
-      end
-
-      it 'redirect_uri cannot be set if api_key is' do
-        authorization = OauthAuthorization.new(api_key: @api_key, redirect_uri: '')
-        expect(authorization).to_not(be_valid)
-        expect(authorization.errors[:redirect_uri]).to(include("must be nil if api_key is present"))
-      end
-
-      it 'validates with api_key' do
-        authorization = OauthAuthorization.new(oauth_app_user: @app_user, api_key: @api_key)
-        expect(authorization).to(be_valid)
-      end
-
-      it 'validates with code' do
-        authorization = OauthAuthorization.new(oauth_app_user: @app_user, code: 'wadus')
-        expect(authorization).to(be_valid)
-      end
-    end
-
-    describe '#exchange!' do
-      before(:all) do
-        @user = FactoryGirl.create(:carto_user)
-        @app = FactoryGirl.create(:oauth_app, user: @user)
-        @app_user = OauthAppUser.create(user: @user, oauth_app: @app)
-      end
-
-      after(:all) do
-        @app_user.destroy
-        @user.destroy
-        @app.destroy
-      end
-
-      before(:each) do
-        @authorization = @app_user.oauth_authorizations.create_with_code!(nil)
-      end
-
-      after(:each) do
-        @authorization.destroy
-      end
-
-      it 'fails if the code is expired' do
-        @authorization.created_at -= 10.minutes
-        expect { @authorization.exchange! }.to(raise_error(OauthProvider::Errors::InvalidGrant))
-        expect(@authorization.code).to(be)
-        expect(@authorization.api_key).to(be_nil)
-      end
-
-      it 'creates a new api key and blanks the code' do
-        @authorization.exchange!
-        expect(@authorization.code).to(be_nil)
-        expect(@authorization.api_key).to(be)
-        expect(@authorization.api_key.type).to(eq('oauth'))
-      end
-    end
-  end
-end