diff --git a/lib/doorkeeper/models/application_mixin.rb b/lib/doorkeeper/models/application_mixin.rb index ccf40406f..9f40961db 100644 --- a/lib/doorkeeper/models/application_mixin.rb +++ b/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. diff --git a/spec/models/doorkeeper/application_spec.rb b/spec/models/doorkeeper/application_spec.rb index d45e0c2b2..09e672091 100644 --- a/spec/models/doorkeeper/application_spec.rb +++ b/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 diff --git a/spec/support/shared/hashing_shared_context.rb b/spec/support/shared/hashing_shared_context.rb index 16a822cd6..1c11b491c 100644 --- a/spec/support/shared/hashing_shared_context.rb +++ b/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