Commit 0e821c0

mo <mo@mokhan.ca>
2018-10-20 18:33:24
to primary key instead of uuid column
1 parent cf9f1dd
app/controllers/scim/v2/groups_controller.rb
@@ -14,7 +14,7 @@ module Scim
       private
 
       def resources
-        User.pluck(:uuid, :email).map do |x|
+        User.pluck(:id, :email).map do |x|
           { id: x[0], userName: x[1] }
         end
       end
app/controllers/scim/v2/users_controller.rb
@@ -17,7 +17,7 @@ module Scim
       end
 
       def show
-        @user = User.find_by!(uuid: params[:id])
+        @user = User.find(params[:id])
         response.headers['Location'] = scim_v2_user_url(@user)
         fresh_when(@user)
         render formats: :scim, status: :ok
app/controllers/oauths_controller.rb
@@ -4,7 +4,7 @@ class OauthsController < ApplicationController
   VALID_RESPONSE_TYPES = %w[code token].freeze
 
   def show
-    @client = Client.find_by!(uuid: secure_params[:client_id])
+    @client = Client.find(secure_params[:client_id])
 
     unless @client.valid_redirect_uri?(secure_params[:redirect_uri])
       return redirect_to @client.redirect_url(
@@ -26,7 +26,7 @@ class OauthsController < ApplicationController
   def create(oauth = session[:oauth])
     return render_error(:bad_request) if oauth.nil?
 
-    client = Client.find_by!(uuid: oauth[:client_id])
+    client = Client.find(oauth[:client_id])
     redirect_to client.redirect_url_for(current_user, oauth)
   rescue StandardError => error
     logger.error(error)
app/controllers/sessions_controller.rb
@@ -41,7 +41,7 @@ class SessionsController < ApplicationController
     if saml_params[:SAMLRequest].present?
       saml = binding.deserialize(saml_params)
       raise ActiveRecord::RecordInvalid.new(saml) if saml.invalid?
-      raise 'Unknown NameId' unless current_user.uuid == saml.name_id
+      raise 'Unknown NameId' unless current_user.to_param == saml.name_id
 
       session[:saml] = { params: saml_params.to_h, xml: saml.to_xml }
       redirect_to response_path
app/controllers/tokens_controller.rb
@@ -25,7 +25,7 @@ class TokensController < ApplicationController
 
   def revoke
     claims = Token.claims_for(params[:token], token_type: :any)
-    Token.find_by(uuid: claims[:jti]).revoke! unless claims.empty?
+    Token.find(claims[:jti]).revoke! unless claims.empty?
     render plain: "", status: :ok
   rescue StandardError => error
     logger.error(error)
@@ -38,7 +38,7 @@ class TokensController < ApplicationController
 
   def authenticate!
     @current_client = authenticate_with_http_basic do |client_id, client_secret|
-      Client.find_by(uuid: client_id)&.authenticate(client_secret)
+      Client.find(client_id)&.authenticate(client_secret)
     end
     return if current_client
 
@@ -61,7 +61,7 @@ class TokensController < ApplicationController
 
   def refresh_grant(refresh_token = params[:refresh_token])
     jti = Token.claims_for(refresh_token, token_type: :refresh)[:jti]
-    token = Token.find_by!(uuid: jti)
+    token = Token.find(jti)
     token.issue_tokens_to(current_client)
   end
 
@@ -77,7 +77,7 @@ class TokensController < ApplicationController
     return if assertion.invalid?
 
     user = if assertion.name_id_format == Saml::Kit::Namespaces::PERSISTENT
-             User.find_by!(uuid: assertion.name_id)
+             User.find(assertion.name_id)
            else
              User.find_by!(email: assertion.name_id)
            end
@@ -103,7 +103,7 @@ class TokensController < ApplicationController
 
   def revoked_tokens
     Rails.cache.fetch("revoked-tokens", expires_in: 10.minutes) do
-      Hash[Token.revoked.pluck(:uuid).map { |x| [x, true] }]
+      Hash[Token.revoked.pluck(:id).map { |x| [x, true] }]
     end
   end
 end
app/models/scim/user.rb
@@ -4,10 +4,9 @@ module SCIM
   class User
     include ActiveModel::Model
     attr_accessor :id, :schemas, :userName, :name, :locale, :timezone, :password
-    UUID_REGEX = /\A\h{8}-\h{4}-\h{4}-\h{4}-\h{12}\z/
 
     validate :must_be_user_schema
-    validates :id, format: { with: UUID_REGEX }, if: proc { |x| x.id.present? }
+    validates :id, format: { with: ApplicationRecord::UUID }, if: proc { |x| x.id.present? }
     validates :locale, presence: true, inclusion: ::User::VALID_LOCALES
     validates :timezone, presence: true, inclusion: ::User::VALID_TIMEZONES
     validates :userName, presence: true, email: true
@@ -15,7 +14,7 @@ module SCIM
     def save!
       validate!
       if id.present?
-        user = ::User.find_by!(uuid: id)
+        user = ::User.find(id)
         ensure_password_update_is_allowed!(user) if password.present?
         user.update!(to_h)
       else
app/models/scim/user_mapper.rb
@@ -8,7 +8,7 @@ module SCIM
 
     def map_from(user)
       Scim::Shady::User.build do |x|
-        x.id = user.uuid
+        x.id = user.id
         x.username = user.email
         x.created_at = user.created_at
         x.updated_at = user.updated_at
app/models/scim/user_repository.rb
@@ -9,7 +9,7 @@ module SCIM
     end
 
     def find!(id)
-      mapper.map_from(::User.find_by!(uuid: id))
+      mapper.map_from(::User.find(id))
     end
 
     def create!(params)
@@ -25,7 +25,7 @@ module SCIM
     end
 
     def update!(id, params)
-      user = ::User.find_by!(uuid: id)
+      user = ::User.find(id)
       user.update!(
         email: params[:userName],
         locale: params[:locale],
@@ -35,7 +35,7 @@ module SCIM
     end
 
     def destroy!(id)
-      ::User.find_by!(uuid: id).destroy!
+      ::User.find(id).destroy!
     end
   end
 end
app/models/client.rb
@@ -12,14 +12,12 @@ class Client < ApplicationRecord
   validates :jwks_uri, format: { with: URI_REGEX }, allow_blank: true
   validates :logo_uri, format: { with: URI_REGEX }, allow_blank: true
   validates :name, presence: true
-  validates :uuid, presence: true, format: { with: UUID }
   validates_each :redirect_uris do |record, attr, value|
     invalid_uri = Array(value).find { |x| !x.match?(URI_REGEX) }
     record.errors[:redirect_uris] << 'is invalid.' if invalid_uri
   end
 
   after_initialize do
-    self.uuid = SecureRandom.uuid unless uuid
     self.password = SecureRandom.base58(24) unless password_digest
   end
 
@@ -36,10 +34,6 @@ class Client < ApplicationRecord
     end
   end
 
-  def to_param
-    uuid
-  end
-
   def valid_redirect_uri?(redirect_uri)
     self.redirect_uris.include? redirect_uri
   end
app/models/token.rb
@@ -12,7 +12,6 @@ class Token < ApplicationRecord
   scope :revoked, -> { where('revoked_at < ?', Time.now) }
 
   after_initialize do |x|
-    x.uuid = SecureRandom.uuid if x.uuid.nil?
     if x.expired_at.nil?
       x.expired_at = access? ? 1.hour.from_now : 1.day.from_now
     end
@@ -32,7 +31,7 @@ class Token < ApplicationRecord
       exp: expired_at.to_i,
       iat: created_at.to_i,
       iss: Saml::Kit.configuration.entity_id,
-      jti: uuid,
+      jti: id,
       nbf: created_at.to_i,
       sub: subject.to_param,
       token_type: token_type,
@@ -66,7 +65,7 @@ class Token < ApplicationRecord
       claims = claims_for(jwt, token_type: :access)
       return if claims.empty?
 
-      token = Token.find_by!(uuid: claims[:jti])
+      token = Token.find(claims[:jti])
       return if token.refresh? || token.revoked?
 
       token
app/models/user.rb
@@ -13,12 +13,8 @@ class User < ApplicationRecord
   validates :timezone, inclusion: VALID_TIMEZONES
   validates :locale, inclusion: VALID_LOCALES
 
-  after_initialize do
-    self.uuid = SecureRandom.uuid unless uuid
-  end
-
   def name_id_for(name_id_format)
-    Saml::Kit::Namespaces::PERSISTENT == name_id_format ? uuid : email
+    Saml::Kit::Namespaces::PERSISTENT == name_id_format ? id : email
   end
 
   def assertion_attributes_for(request)
@@ -37,10 +33,6 @@ class User < ApplicationRecord
     Mfa.new(self)
   end
 
-  def to_param
-    uuid
-  end
-
   class << self
     def login(email, password)
       return if email.blank? || password.blank?
@@ -55,6 +47,6 @@ class User < ApplicationRecord
   private
 
   def trusted_attributes_for(_request)
-    { id: uuid, email: email, created_at: created_at }
+    { id: id, email: email, created_at: created_at }
   end
 end
app/views/scim/v2/users/_user.scim.jbuilder
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 json.schemas ["urn:ietf:params:scim:schemas:core:2.0:User"]
-json.id @user.uuid
+json.id @user.to_param
 json.meta do
   json.resourceType 'User'
   json.created @user.created_at.iso8601
config/initializers/configuration.rb
@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
 config = Rails.application.config
+config.generators do |g|
+  g.orm :active_record, primary_key_type: :uuid
+end
 config.x.jwt.private_key = OpenSSL::PKey::RSA.new(2048)
 
 I18n.available_locales = [:en, :es, :fr, :ja, :ko]
db/migrate/20170101000000_enable_pg_extensions.rb
@@ -0,0 +1,6 @@
+class EnablePgExtensions < ActiveRecord::Migration[5.2]
+  def change
+    enable_extension 'uuid-ossp'
+    enable_extension 'pgcrypto'
+  end
+end
db/migrate/20171021193946_create_users.rb
@@ -2,9 +2,8 @@
 
 class CreateUsers < ActiveRecord::Migration[5.1]
   def change
-    create_table :users do |t|
+    create_table :users, id: :uuid do |t|
       t.string :email
-      t.string :uuid, null: false, index: true
       t.string :password_digest
       t.timestamps null: false
     end
db/migrate/20180905011437_create_clients.rb
@@ -2,8 +2,7 @@
 
 class CreateClients < ActiveRecord::Migration[5.2]
   def change
-    create_table :clients do |t|
-      t.string :uuid, null: false, index: true
+    create_table :clients, id: :uuid do |t|
       t.string :name, null: false
       t.string :password_digest, null: false
       t.string :redirect_uri, null: false
db/migrate/20180905020708_create_authorizations.rb
@@ -2,9 +2,9 @@
 
 class CreateAuthorizations < ActiveRecord::Migration[5.2]
   def change
-    create_table :authorizations do |t|
-      t.references :user, foreign_key: true
-      t.references :client, foreign_key: true
+    create_table :authorizations, id: :uuid do |t|
+      t.references :user, foreign_key: true, type: :uuid
+      t.references :client, foreign_key: true, type: :uuid
       t.string :code, null: false, index: true
       t.string :challenge
       t.integer :challenge_method, default: 0
db/migrate/20180909173139_create_tokens.rb
@@ -2,11 +2,10 @@
 
 class CreateTokens < ActiveRecord::Migration[5.2]
   def change
-    create_table :tokens do |t|
-      t.string :uuid, index: { unique: true }
+    create_table :tokens, id: :uuid do |t|
       t.references :authorization
-      t.references :subject, polymorphic: true
-      t.references :audience, polymorphic: true
+      t.references :subject, polymorphic: true, type: :uuid
+      t.references :audience, polymorphic: true, type: :uuid
       t.integer :token_type, default: 0
       t.datetime :expired_at
       t.datetime :revoked_at
db/migrate/20180922153546_create_user_sessions.rb
@@ -2,8 +2,8 @@
 
 class CreateUserSessions < ActiveRecord::Migration[5.2]
   def change
-    create_table :user_sessions do |t|
-      t.references :user, foreign_key: true
+    create_table :user_sessions, id: :uuid do |t|
+      t.references :user, foreign_key: true, type: :uuid
       t.string :key
       t.string :ip
       t.text :user_agent
db/migrate/20180923222720_install_audited.rb
@@ -2,12 +2,12 @@
 
 class InstallAudited < ActiveRecord::Migration[5.2]
   def self.up
-    create_table :audits, force: true do |t|
-      t.column :auditable_id, :integer
+    create_table :audits, id: :uuid do |t|
+      t.column :auditable_id, :string
       t.column :auditable_type, :string
-      t.column :associated_id, :integer
+      t.column :associated_id, :string
       t.column :associated_type, :string
-      t.column :user_id, :integer
+      t.column :user_id, :string
       t.column :user_type, :string
       t.column :username, :string
       t.column :action, :string
db/schema.rb
@@ -13,14 +13,16 @@
 ActiveRecord::Schema.define(version: 2018_10_20_161349) do
 
   # These are extensions that must be enabled in order to support this database
+  enable_extension "pgcrypto"
   enable_extension "plpgsql"
+  enable_extension "uuid-ossp"
 
-  create_table "audits", force: :cascade do |t|
-    t.integer "auditable_id"
+  create_table "audits", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+    t.string "auditable_id"
     t.string "auditable_type"
-    t.integer "associated_id"
+    t.string "associated_id"
     t.string "associated_type"
-    t.integer "user_id"
+    t.string "user_id"
     t.string "user_type"
     t.string "username"
     t.string "action"
@@ -37,9 +39,9 @@ ActiveRecord::Schema.define(version: 2018_10_20_161349) do
     t.index ["user_id", "user_type"], name: "index_audits_on_user_id_and_user_type"
   end
 
-  create_table "authorizations", force: :cascade do |t|
-    t.bigint "user_id"
-    t.bigint "client_id"
+  create_table "authorizations", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+    t.uuid "user_id"
+    t.uuid "client_id"
     t.string "code", null: false
     t.string "challenge"
     t.integer "challenge_method", default: 0
@@ -52,8 +54,7 @@ ActiveRecord::Schema.define(version: 2018_10_20_161349) do
     t.index ["user_id"], name: "index_authorizations_on_user_id"
   end
 
-  create_table "clients", force: :cascade do |t|
-    t.string "uuid", null: false
+  create_table "clients", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.string "name", null: false
     t.string "password_digest", null: false
     t.datetime "created_at", null: false
@@ -62,7 +63,6 @@ ActiveRecord::Schema.define(version: 2018_10_20_161349) do
     t.integer "token_endpoint_auth_method", default: 0, null: false
     t.string "logo_uri"
     t.string "jwks_uri"
-    t.index ["uuid"], name: "index_clients_on_uuid"
   end
 
   create_table "flipper_features", force: :cascade do |t|
@@ -90,13 +90,12 @@ ActiveRecord::Schema.define(version: 2018_10_20_161349) do
     t.index ["updated_at"], name: "index_sessions_on_updated_at"
   end
 
-  create_table "tokens", force: :cascade do |t|
-    t.string "uuid"
+  create_table "tokens", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.bigint "authorization_id"
     t.string "subject_type"
-    t.bigint "subject_id"
+    t.uuid "subject_id"
     t.string "audience_type"
-    t.bigint "audience_id"
+    t.uuid "audience_id"
     t.integer "token_type", default: 0
     t.datetime "expired_at"
     t.datetime "revoked_at"
@@ -105,11 +104,10 @@ ActiveRecord::Schema.define(version: 2018_10_20_161349) do
     t.index ["audience_type", "audience_id"], name: "index_tokens_on_audience_type_and_audience_id"
     t.index ["authorization_id"], name: "index_tokens_on_authorization_id"
     t.index ["subject_type", "subject_id"], name: "index_tokens_on_subject_type_and_subject_id"
-    t.index ["uuid"], name: "index_tokens_on_uuid", unique: true
   end
 
-  create_table "user_sessions", force: :cascade do |t|
-    t.bigint "user_id"
+  create_table "user_sessions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+    t.uuid "user_id"
     t.string "key"
     t.string "ip"
     t.text "user_agent"
@@ -122,9 +120,8 @@ ActiveRecord::Schema.define(version: 2018_10_20_161349) do
     t.index ["user_id"], name: "index_user_sessions_on_user_id"
   end
 
-  create_table "users", force: :cascade do |t|
+  create_table "users", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.string "email"
-    t.string "uuid", null: false
     t.string "password_digest"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
@@ -132,7 +129,6 @@ ActiveRecord::Schema.define(version: 2018_10_20_161349) do
     t.string "mfa_secret"
     t.string "locale", default: "en", null: false
     t.string "timezone", default: "Etc/UTC", null: false
-    t.index ["uuid"], name: "index_users_on_uuid"
   end
 
   add_foreign_key "authorizations", "clients"
spec/factories/client.rb
@@ -2,7 +2,6 @@
 
 FactoryBot.define do
   factory :client do
-    uuid { SecureRandom.uuid }
     name { FFaker::Name.name }
     redirect_uris { [FFaker::Internet.uri('https')] }
   end
spec/factories/token.rb
@@ -2,7 +2,6 @@
 
 FactoryBot.define do
   factory :token do
-    uuid { SecureRandom.uuid }
     authorization { nil }
     association :audience, factory: :client
     association :subject, factory: :user
spec/factories/user.rb
@@ -3,7 +3,6 @@
 FactoryBot.define do
   factory :user do
     email { FFaker::Internet.email }
-    uuid { SecureRandom.uuid }
     password { FFaker::Internet.password }
 
     trait :mfa_configured do
spec/models/scim/user_spec.rb
@@ -60,7 +60,6 @@ RSpec.describe SCIM::User do
         subject { build(:scim_user) }
 
         specify { expect(subject.save!).to be_persisted }
-        specify { expect(subject.save!.uuid).to be_present }
         specify { expect(subject.save!.email).to eql(subject.userName) }
         specify { expect(subject.save!.locale).to eql(subject.locale) }
         specify { expect(subject.save!.timezone).to eql(subject.timezone) }
@@ -76,7 +75,7 @@ RSpec.describe SCIM::User do
 
       before { allow(Current).to receive(:user).and_return(current_user) }
 
-      specify { expect(subject.save!.uuid).to eql(other_user.uuid) }
+      specify { expect(subject.save!.to_param).to eql(other_user.to_param) }
       specify { expect(subject.save!.email).to eql(subject.userName) }
       specify { expect(subject.save!.locale).to eql(subject.locale) }
       specify { expect(subject.save!.timezone).to eql(subject.timezone) }
spec/models/client_spec.rb
@@ -10,8 +10,6 @@ RSpec.describe Client do
     specify { expect(build(:client, redirect_uris: ['<script>alert("hi")</script>'])).to be_invalid }
     specify { expect(build(:client, redirect_uris: ['invalid'])).to be_invalid }
     specify { expect(build(:client, redirect_uris: 'invalid')).to be_invalid }
-    specify { expect(build(:client, uuid: nil)).to be_invalid }
-    specify { expect(build(:client, uuid: 'invalid')).to be_invalid }
     specify { expect(build(:client, name: nil)).to be_invalid }
   end
 
spec/requests/scim/v2/groups_spec.rb
@@ -25,7 +25,7 @@ describe "/scim/v2/groups" do
 
       specify { expect(json[:schemas]).to match_array([Scim::Shady::Messages::LIST_RESPONSE]) }
       specify { expect(json[:totalResults]).to be_kind_of(Numeric) }
-      specify { expect(json[:Resources]).to match_array([id: user.uuid, userName: user.email]) }
+      specify { expect(json[:Resources]).to match_array([id: user.to_param, userName: user.email]) }
     end
   end
 
spec/requests/scim/v2/users_spec.rb
@@ -58,7 +58,7 @@ describe '/scim/v2/users' do
     let(:user) { create(:user) }
 
     context "when the resource is available" do
-      before { get "/scim/v2/users/#{user.uuid}", headers: headers }
+      before { get "/scim/v2/users/#{user.to_param}", headers: headers }
 
       let(:json) { JSON.parse(response.body, symbolize_names: true) }
 
@@ -69,7 +69,7 @@ describe '/scim/v2/users' do
       specify { expect(response.body).to be_present }
 
       specify { expect(json[:schemas]).to match_array([Scim::Shady::Schemas::USER]) }
-      specify { expect(json[:id]).to eql(user.uuid) }
+      specify { expect(json[:id]).to eql(user.to_param) }
       specify { expect(json[:userName]).to eql(user.email) }
       specify { expect(json[:meta][:resourceType]).to eql('User') }
       specify { expect(json[:meta][:created]).to eql(user.created_at.iso8601) }
@@ -128,7 +128,7 @@ describe '/scim/v2/users' do
     let(:body) { { schemas: [Scim::Shady::Schemas::USER], userName: new_email, locale: locale, timezone: timezone } }
     let(:json) { JSON.parse(response.body, symbolize_names: true) }
 
-    before { put "/scim/v2/users/#{user.uuid}", headers: headers, params: body.to_json }
+    before { put "/scim/v2/users/#{user.to_param}", headers: headers, params: body.to_json }
 
     specify { expect(response).to have_http_status(:ok) }
     specify { expect(response.headers['Content-Type']).to eql('application/scim+json') }
@@ -151,11 +151,11 @@ describe '/scim/v2/users' do
     let(:other_user) { create(:user) }
 
     context "when the user can be deleted" do
-      before { delete "/scim/v2/users/#{other_user.uuid}", headers: headers }
+      before { delete "/scim/v2/users/#{other_user.to_param}", headers: headers }
 
       specify { expect(response).to have_http_status(:no_content) }
       specify do
-        get "/scim/v2/users/#{other_user.uuid}", headers: headers
+        get "/scim/v2/users/#{other_user.to_param}", headers: headers
         expect(response).to have_http_status(:not_found)
       end
     end
spec/requests/clients_spec.rb
@@ -24,7 +24,7 @@ RSpec.describe "/clients" do
       specify { expect(response.headers['Content-Type']).to include("application/json") }
       specify { expect(response.headers['Cache-Control']).to include("no-store") }
       specify { expect(response.headers['Pragma']).to eql("no-cache") }
-      specify { expect(json[:client_id]).to eql(last_client.uuid) }
+      specify { expect(json[:client_id]).to eql(last_client.to_param) }
       specify { expect(json[:client_secret]).to be_present }
       specify { expect(json[:client_id_issued_at]).to eql(last_client.created_at.to_i) }
       specify { expect(json[:client_secret_expires_at]).to be_zero }
spec/requests/tokens_spec.rb
@@ -4,7 +4,7 @@ require 'rails_helper'
 
 RSpec.describe '/tokens' do
   let(:client) { create(:client) }
-  let(:credentials) { ActionController::HttpAuthentication::Basic.encode_credentials(client.uuid, client.password) }
+  let(:credentials) { ActionController::HttpAuthentication::Basic.encode_credentials(client.to_param, client.password) }
   let(:headers) { { 'Authorization' => credentials } }
 
   describe "POST /oauth/token" do