Problematic Default Configuration for Devise.secret_key

[luna-lightblade]

Currently the default behavior when generating a Solidus application using the generator and including Solidus Auth Devise is to set the Devise.secret_key to a random value at application boot.

This is handled by lib/generators/solidus/auth/install/templates/config/initializers/devise.rb and creates the following code in config/initializers/devise.rb.

# frozen_string_literal: true

Devise.secret_key = SecureRandom.hex(50).inspect

This creates problematic behavior, because the secret key base used to generate password reset tokens uses an ephemeral key which is lost on application reboot, and is not known across different instances of the same application (for example multiple Kubernetes pods running the app). As a result, all password reset links are invalidated when the application is restarted, and password reset links will fail to function if a different replica of the application handles the request to set the password from the replica that initiated the reset.

By default, Devise uses Rails.application.secret_key_base if you do not set Devise.secret_key explicitly which is generally stable across restarts and (if correctly configured) different replicas of the application. Accepting this as the default behavior by not setting Devise.secret_key at all would prevent these problems that the default behavior currently produces.

Solidus Version: 4.3.4

To Reproduce
The behavior is reproduced when creating a new solidus application using the solidus generator.

[jarednorman]

I feel like there must be more context here, or all our apps using this would have encountered issues with their passwords not working.

[fthobe]

@luna-lightblade were you able to reproduce the problem on new installations?

[fthobe]

@luna-lightblade I did reproduce the problem and what you said makes absolute sense.

The thing is this one: while a staging deployment might be restarted frequently, a production one is not, the multi instance point is very valid though.

@jarednorman this is no desired behavior, but not an issue neither if solidus is not spread across multiple instances. This is not a safety issue, but it is a valid question to either document limitations on multi instance scenarios or fix this and write the hash to generate the key into db.

[fthobe]

@jarednorman this is a legitimate bug in multi server scenarios, we should generate this key once and document that it's needed to replicate it when using multiple instances with shared DB.

Do you know if there are more similar encryption keys that are temporary in nature?

[jarednorman]

I've only run Solidus in multi-instance scenarios, which leaves me very confused as to why this hasn't been an issue. Regardless, I think we should remove this. It doesn't make any sense to me.

[fthobe]

I think it's very hard to catch depending on your load balancer / rev proxy config as it might present intermittently if you do round robin or only on fall back during the window of validity of the token.

Should it be written into db?

[fthobe]

I've only run Solidus in multi-instance scenarios, which leaves me very confused as to why this hasn't been an issue.

Could you check what you do on multi instance scenarios or do you have only fall back scenarios? It wouldn't present till a failover from one instance to another.

[fthobe]

@jarednorman ?

[jarednorman]

I'm not talking about fallback scenarios. All our stores run multiple instances that are load balanced. I suspect people just end up fixing this issue.

The correct configuration, as per devise itself (see here), is something like:

Devise.secret_key = '<%= SecureRandom.hex(64) %>'

People should probably be putting a per-environment secret value in here using something like Rails credentials, but as a default, ensuring that we're interpolating a random value in here (rather than generating a new one each boot) is probably a fine solution.

[fthobe]

@jarednorman Given that this is not a heavy hitter from a traffic perspective we should consider throwing this into the db, what do you think?

[jarednorman]

I don't know what you mean by that.

We should restore the configuration that Devise uses and stores that (rightly) want to store the value somewhere else can move it to Rails credentials or some other secure storage.

[fthobe]

I don't know what you mean by that.

I don't see recovery emails being very resource intensive. Maybe we should consider storing them in the DB so that all instances have access in any way.

The social login extension stores the credentials in admin, I don't see why the recovery key would be particularily more sensitive, but I can imagine that's not a universal position.

[jarednorman]

I see no reason to diverge from Devise defaults for this. If individual stores want to do something more complex involving the database, that's on them.