search

lynndylanhurley / devise_token_auth

Token based authentication for Rails JSON APIs. Designed to work with jToker and ng-token-auth.
Do What The F*ck You Want To Public License
3.54k stars 1.13k forks source link

DB serialization issue in Rails 5.2.0 #1079

Open Marinlemaignan opened 6 years ago

Marinlemaignan commented 6 years ago

Unfortunately, upgrading to Rails 5.2.0 (precisely Rails 5.2.0.rc1) with devise_token_auth (0.1.43.beta1) is impossible.

I cannot even manage to migrate my test database anymore.

zachfeldman commented 6 years ago

@Marinlemaignan sorry to hear that. Would you like to work on a pull request to fix this problem?

MaicolBen commented 6 years ago

Weird, it didn't happen to me with devise_token_auth 0.1.43.beta1 and master. So you run rake db:migrate, right? That's weird because table_exists? should be false, can you override that method and check which condition of tokens_has_json_column_type? is trowing false?

Marinlemaignan commented 6 years ago

Hey, so ! got a little further. Here's more details, this happens when i run something like: rails db:environment:set RAILS_ENV=test && rails db:drop RAILS_ENV=test && rails db:create RAILS_ENV=test && rails db:migrate RAILS_ENV=test

  1. when the db is being dropped, tokens_has_json_column_type? = true -> All fine
  2. when the db is created, but the Users table isn't yet there, or the tokens attribute hasn't yet been defined, or hasn't yet been redefined as json (im my case, the users table exists but the tokens column isn't there yet because token auth was added later on), tokens_has_json_column_type? = false -> Migration starts, and as soon as the users table gets the tokens attribute and is modified, the error gets raised

And yes, @MaicolBen table_exists? returns false so tokens_has_json_column_type? returns false as well, and therefore the unless tokens_has_json_column_type? lets serialize :tokens being applied to the User class. am i right ?

My guess, this might behave differently depending on Class initialization / load order? Also, overriding tokens_has_json_column_type? at the user model level has no effect so the entire file has to be copied over to the app/models/concerns/ folder (i bet due to the same reasons as above)

zachfeldman commented 6 years ago

It makes sense to me that you'd have this issue in that scenario as @MaicolBen may be approaching it from an existing installation.

@Marinlemaignan can you think of a fix for this scenario?

Marinlemaignan commented 6 years ago

I'm rather confused, but maybe it'd be best to leave that option up to the end-user. Is an error raised if the column is :text and the serialize option is not defined on the Class? So.. maybe flipping the problem the other way round, and raising some error if the column needs to be serialized, something like DeviseTokenAuth::ColumnNeedsSerializationError with a simple message prompting you to add the serialize :tokens bit to your model ? But there would definitely be a better way to go...

MaicolBen commented 6 years ago

~It's an easy fix, it should be checking if the field is present or not~

It's already checking for the field presence, which is weird. I will figure out what could be happening

MaicolBen commented 6 years ago

@Marinlemaignan I have the tokens migration added later as well, so I guess you have code after the migration doing something with the User, can you post the migration?

Marinlemaignan commented 6 years ago

well actually i kinda forgot, but my colleagues squashed the first (huge batch) of migrations (our old v1 basically) into a giant snapshot of it's state right before v2. nevermind that makes no difference (even worse, because when i run RAILS_ENV=test rails db:create the :tokens column is already defined), then about 15-20 migrations later we're doing work on the user's model to which we add another column (here i name it :something)

def up
    User.connection.schema_cache.clear!
    User.reset_column_information

    User.where.not(something: 'something').each do |u|
      u.something = 'nothing'
      u.save!
    end
end

That's all. Also i constantly drop and recreate my test_db.

Maybe using User.reload! in my migration could help but that doesnt seem like a correct enough fix to me.

MaicolBen commented 6 years ago

Yep, you're right, I could reproduce it with your context, the problem is self.columns_hash['tokens'], it isn't in the model yet, but in the database. I will find an alternative way to check the presence of it in the model.

Marinlemaignan commented 6 years ago

This is great ! 👍

MaicolBen commented 6 years ago

The workaround that you can do is putting the next code inside the migration class:

  class User < ApplicationRecord
    def tokens_has_json_column_type?
      false
    end
  end

But we can't do that in the gem

Marinlemaignan commented 6 years ago

Yes that's kinda like the User.reload! i mentioned earlier

dks17 commented 6 years ago

I could not reproduce it. Try this code:

def tokens_has_json_column_type?
  # database_exists? && table_exists? && self.columns_hash['tokens'] && self.columns_hash['tokens'].type.in?([:json, :jsonb])
  database_exists? && table_exists? && self.type_for_attribute('tokens').type.in?([:json, :jsonb])
end
houen commented 6 years ago

I had this as well, and only when dropping the database before running db:setup. To reproduce:

bundle exec rails db:drop && bundle exec rails db:setup
...
...
rails aborted!
ActiveRecord::AttributeMethods::Serialization::ColumnNotSerializableError: Column `tokens` of type ActiveRecord::Type::Json does not support `serialize` feature.
Usually it means that you are trying to use `serialize`
on a column that already implements serialization natively.

It seems that (at least in my case) the issue is something Rails first picks up on boot after the database has been migrated. I have seen this happen with db:setup in the past with unrelated issues as well.

In contrast, for me everything works fine if I split up the calls between db:migrate and db:seed so Rails reboots:

bundle exec rails db:drop db:create db:migrate && bundle exec rails db:seed

This works without issues.

hcyildirim commented 6 years ago

Hi,

This is also the case, when I delete the database and re-create it and run the tests, the first running test fails. When I run it for the second time, all the tests pass.

Is downgrade works?

Thank you.

codingwaysarg commented 6 years ago

I'm having the same issue than @ccoeder

First test fails, second test pass. Weird.

hcyildirim commented 6 years ago

After 2 months, its not working again. I don't know what to do. Is there any solutions? @houen I tried your solution but its not working for me.

luisalima commented 6 years ago

Suddenly stopped working for me too. I didn't upgrade, it seems like a random fail, only in test environment.

dks17 commented 6 years ago

I am working on mongoid implementation for the gem #1198 . And I have encountered with failed tests. I.g. they changed one of validator behavior. I suggest you use 5-2-stable branch or 5.1 version of rails.

Bukenco commented 6 years ago

any solution?

typhoon2099 commented 6 years ago

Our tokens fields uses type t.json. I notice the test schema uses t.text. Might this be relevant?

fourcolors commented 5 years ago

I have a t.json type but when I try to retrieve it ie: Map.objects it's returning a String instead of a Hash of objects.

This only started happening in Rails 5.2.1 for me. Any chance this is related? I haven't found anyone talking about this.

I know this isn't related to devise but related to this issue. I'm not sure where the right place to post this would be.

bernabe9 commented 5 years ago

@MaicolBen any updates on this?

MaicolBen commented 5 years ago

Hey @bernabe9! Try my workaround above in the migration, we cannot put that in the code for obvious reasons, I should check if this still happening with rails 5.2 because @dks17 can't reproduce it anymore.

zachfeldman commented 5 years ago

Can confirm that upon updating my app to Rails 5.2 this threw in a few migrations. @MaicolBen your workaround fixed it!

vjdavid commented 5 years ago

I am sure that this is a random error, because I've already ran the migrations without the workaround but suddenly the error no longer appear at this moment with Rails 2.5.1

dkniffin commented 5 years ago

I'm still seeing this same error when generating a new rails app with 5.2

MaicolBen commented 5 years ago

Yeah, it's confirmed, don't need to post here, it happens when you run together 2 migrations (you don't boot the env between them) where one of them is the one that you add the DTA's fields and the other one is modifying users.

dkniffin commented 5 years ago

In my case, I only have the one DTA migration

class DeviseTokenAuthCreateUsers < ActiveRecord::Migration[5.2]
  def change

    create_table(:users) do |t|
      ## Required
      t.string :provider, :null => false, :default => "email"
      t.string :uid, :null => false, :default => ""

      ## Database authenticatable
      t.string :encrypted_password, :null => false, :default => ""

      ## Recoverable
      t.string   :reset_password_token
      t.datetime :reset_password_sent_at
      t.boolean  :allow_password_change, :default => false

      ## Rememberable
      t.datetime :remember_created_at

      ## Trackable
      t.integer  :sign_in_count, :default => 0, :null => false
      t.datetime :current_sign_in_at
      t.datetime :last_sign_in_at
      t.string   :current_sign_in_ip
      t.string   :last_sign_in_ip

      ## Confirmable
      t.string   :confirmation_token
      t.datetime :confirmed_at
      t.datetime :confirmation_sent_at
      t.string   :unconfirmed_email # Only if using reconfirmable

      ## Lockable
      # t.integer  :failed_attempts, :default => 0, :null => false # Only if lock strategy is :failed_attempts
      # t.string   :unlock_token # Only if unlock strategy is :email or :both
      # t.datetime :locked_at

      ## User Info
      t.string :name
      t.string :nickname
      t.string :image
      t.string :email

      ## Tokens
      t.json :tokens

      t.timestamps
    end

    add_index :users, :email,                unique: true
    add_index :users, [:uid, :provider],     unique: true
    add_index :users, :reset_password_token, unique: true
    add_index :users, :confirmation_token,   unique: true
    # add_index :users, :unlock_token,       unique: true
  end
end
kinsbrunner commented 5 years ago

I am facing the same issue. This is definitelly an issue related to Rails 5.2. Teh quickest workaround I have found is to create a dummy entry through Rails console and delete it, after this, no more errors!

ktmouk commented 5 years ago

In my case, This code works fine. but token is saved in YAML format :confused:

class User < ApplicationRecord
  include DeviseTokenAuth::Concerns::User
  serialize :tokens
   ....
end
dks17 commented 5 years ago

Hello. There is #1250 PR that should fix this issue. Would you help testing it on your projects?

rreusser commented 5 years ago

I had a lot of trouble with this and didn't have luck with any of the above fixes, but after upgrading from rails 5.2.0 to 5.2.2, it seems to succeed.

MaicolBen commented 5 years ago

@rreusser did you try with #1250 ?

rreusser commented 5 years ago

@MaicolBen Apologies. I should clarify. That was the one thing I didn't try. Let's see… if I have some time I can try downgrading and test installing from this repo since obviously not everyone has the luxury of being able to upgrade 😄

MaicolBen commented 5 years ago

This was fixed in #1250, post here if it's still happening when you have this gem from the master branch

gducoder commented 5 years ago

Still happening on rails 5.2

randomprimate commented 5 years ago

This issue is still relevant:

MaicolBen commented 5 years ago

Post more details of the error! This is very hard to fix 😞

micred commented 4 years ago

Please give 2 important informations: 1) which type has the column :tokens in your schema.rb? 2) do you have serialize :tokens in your model?