search

mikker / passwordless

🗝 Authentication for your Rails app without the icky-ness of passwords
MIT License
1.26k stars 87 forks source link

Using an authenticatable with ids that are not ints doesn't work #91

Closed anaulin closed 3 years ago

anaulin commented 3 years ago

We are trying to set up passwordless (which looks sweet and exactly what we need!) with an authenticable model that uses uuid for its id. Because the type of authenticatable_id in the passwordless_sessions is set to bigint, this results in our uuids being cast to integers, which leads to sessions not working, because the authenticatable id no longer matches an existing authenticatable.

I looked through the documentation but did not see a way to configure passwordless to use a uuid string type for its authenticatable_id. We could migrate passwordless_sessions to use a string for its authenticatable_id column, but that feels a little dirty, and I don't know if it will break anything else.

Is there a way to configure passwordless to use uuids as the authenticatable id?

Thx in advance. 🙏

anaulin commented 3 years ago

PS I would be willing to submit a PR to make the id type configurable on setup. If that's something you'd like, let me know if you have a preferred approach to implement, or if you'd prefer me to make a proposal.

mikker commented 3 years ago

Hi @anaulin. Would it work if you just changed the column type? I don't immediately find that dirty at all, fwiw.

How else would we configure it? I think that's the only place we reference the type of to field?

PRs are very welcome, thanks for bringing this up.

anaulin commented 3 years ago

At first glance, it seems to work if we just migrate that column to be a :uuid after the passwordless migration runs. (This does require also dropping an index and recreating it.) I was saying that it feels a little "dirty" or brittle to do this because, not knowing the internals of passwordless itself, I don't know if there might be some other internal piece of passwordless that might be relying on the id being an int. But if you are reassuring me that this is the way to go, then I feel ok with it, so thanks for that! 🙏

Researching a bit more, it looks like the underlying issue is that the passwordless migration is using t.belongs_to and there is a known behavior where Rails' t.belongs_to does not respect the foreign key type when using of the table (see: https://github.com/rails/rails/issues/23422). Seems like a possible solution would be to use t.references (instead of t.belongs), which allows you to set the key type, and to allow passwordless' users to set a type other than int if they need it.

For now, if that's ok with you, I'll make a quick patch to document this behavior in the passwordless README, in case other folks run into this problem.

gruschis commented 3 years ago

Rails 6.0.3.4 and PostgreSQL 12.3

I have set the type in the (passwordless) generated migration file like so:

      t.belongs_to(
        :authenticatable,
        polymorphic: true,
        index: { name: 'authenticatable' },
        type: :uuid
      )

After running db:migrate, the authenticatable_id has the correct type (uuid).

I also changed belongs_to to references which works as well.

mikker commented 3 years ago

Closed by https://github.com/mikker/passwordless/pull/92