[openstreetmap/openstreetmap-website] Drop support for legacy MD5 passwords (PR #7048)

[Pablo Brasero]

@pablobm commented on this pull request.



> @@ -40,6 +34,10 @@ def self.upgrade?(hash, _salt)
     true
   end
 
+  def self.valid?(hash, salt)
+    Argon2::HashFormat.valid_hash?(hash) || salt&.include?("!")

What's the second part about? `salt&.include?("!")` It doesn't seem to be covered by tests.

> @@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class DropMd5Passwords < ActiveRecord::Migration[8.1]
+  def up
+    User
+      .where("pass_crypt SIMILAR TO '[0-9a-z]{32}'")
+      .in_batches
+      .update_all(:pass_crypt => "expired password", :pass_salt => nil)
+  end
+end

Typically I would have this as a separate script, run manually, as opposed to a migration. It's a data migration (not schema), it's not strictly required for the deployment (I think?), and it could potentially run very slowly.

Also, TIL about `in_batches.update_all`. I had a look and it does appear to do the right thing 👍 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7048#pullrequestreview-4203790068
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/7048/review/4203790068 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260430/f4b88c61/attachment.htm>