Fix hash_application_secrets fallback not upgrading secret on access

lib/doorkeeper/models/application_mixin.rb

@@ -27,9 +27,23 @@ def by_uid_and_secret(uid, secret)
         app = by_uid(uid)
         return unless app
         return app if secret.blank? && !app.confidential?
-        return unless app.secret_matches?(secret)
 
-        app
+        # Try the primary (active) secret strategy first.
+        if !secret.nil? && !app.secret.nil? &&
+            secret_strategy.secret_matches?(secret.to_s, app.secret.to_s)
+          return app
+        end
+
+        # Fall back to the previous strategy if configured, upgrading on success
+        # to migrate the stored secret to the active strategy (mirrors the
+        # find_by_plaintext_token -> find_by_fallback_token pattern).
+        if fallback_secret_strategy && !secret.nil? && !app.secret.nil? &&
+            fallback_secret_strategy.secret_matches?(secret.to_s, app.secret.to_s)
+          upgrade_fallback_value(app, :secret, secret)
+          return app
+        end
+
+        nil
       end
 
       # Returns an instance of the Doorkeeper::Application with specific UID.

spec/models/doorkeeper/application_spec.rb

@@ -239,6 +239,33 @@ def self.generate
       end
     end
 
+    context "with application hashing fallback enabled" do
+      include_context "with application hashing and fallback lookup enabled"
+
+      let(:plain_secret) { "plain text secret" }
+
+      before do
+        # Simulate an application that was stored with a plain secret before hashing was enabled
+        app.update_column(:secret, plain_secret)
+      end
+
+      it "upgrades a plain secret when falling back to it" do
+        # Side-effect: This will automatically upgrade the secret
+        expect(described_class).to receive(:upgrade_fallback_value).and_call_original
+        lookup = described_class.by_uid_and_secret(app.uid, plain_secret)
+        expect(lookup).to eq(app)
+
+        # The secret should now be stored as a SHA256 hash
+        app.reload
+        expect(app.secret).not_to eq(plain_secret)
+        expect(app.secret).to eq(Doorkeeper::SecretStoring::Sha256Hash.transform_secret(plain_secret))
+
+        # Will find subsequently by hashing the secret
+        lookup = described_class.by_uid_and_secret(app.uid, plain_secret)
+        expect(lookup).to eq(app)
+      end
+    end
+
     it "does not provide access to secret after loading" do
       lookup = described_class.by_uid_and_secret(app.uid, app.plaintext_secret)
       expect(lookup.plaintext_secret).to be_nil

spec/support/shared/hashing_shared_context.rb

@@ -38,3 +38,16 @@
     end
   end
 end
+
+shared_context "with application hashing and fallback lookup enabled" do
+  let(:hashed_or_plain_token_func) do
+    Doorkeeper::SecretStoring::Sha256Hash.method(:transform_secret)
+  end
+
+  before do
+    Doorkeeper.configure do
+      orm DOORKEEPER_ORM
+      hash_application_secrets fallback: :plain
+    end
+  end
+end