using I18n for all error comparisons

NEWS.md

@@ -7,7 +7,11 @@ User-visible changes worth mentioning.
 
 ## master
 
+- [#PR] Add your PR description here.
 - [#1238] Better support for native app with support for custom scheme and localhost redirection.
+- [#1249]: Specify case sensitive uniqueness to remove Rails 6 deprecation message
+- [#1248] Display the Application Secret in HTML after creating a new application even when `hash_application_secrets` is used.
+- [#1248] Return the unhashed Application Secret in the JSON response after creating new application even when `hash_application_secrets` is used.
 
 ## 5.1.0
 

app/controllers/doorkeeper/applications_controller.rb

@@ -4,6 +4,7 @@ module Doorkeeper
   class ApplicationsController < Doorkeeper::ApplicationController
     layout "doorkeeper/admin" unless Doorkeeper.configuration.api_only
 
+    add_flash_types :application_secret
     before_action :authenticate_admin!
     before_action :set_application, only: %i[show edit update destroy]
 
@@ -32,6 +33,7 @@ def create
 
       if @application.save
         flash[:notice] = I18n.t(:notice, scope: %i[doorkeeper flash applications create])
+        flash[:application_secret] = @application.plaintext_secret
 
         respond_to do |format|
           format.html { redirect_to oauth_application_url(@application) }

app/views/doorkeeper/applications/show.html.erb

@@ -8,7 +8,7 @@
     <p><code class="bg-light" id="application_id"><%= @application.uid %></code></p>
 
     <h4><%= t('.secret') %>:</h4>
-    <p><code class="bg-light" id="secret"><%= @application.plaintext_secret %></code></p>
+    <p><code class="bg-light" id="secret"><%= flash[:application_secret].presence || @application.plaintext_secret %></code></p>
 
     <h4><%= t('.scopes') %>:</h4>
     <p><code class="bg-light" id="scopes"><%= @application.scopes.presence || raw('&nbsp;') %></code></p>

lib/doorkeeper/orm/active_record/access_grant.rb

@@ -16,7 +16,7 @@ class AccessGrant < ActiveRecord::Base
               :redirect_uri,
               presence: true
 
-    validates :token, uniqueness: true
+    validates :token, uniqueness: { case_sensitive: true }
 
     before_validation :generate_token, on: :create
 

lib/doorkeeper/orm/active_record/access_token.rb

@@ -9,8 +9,8 @@ class AccessToken < ActiveRecord::Base
     belongs_to :application, class_name: "Doorkeeper::Application",
                              inverse_of: :access_tokens, optional: true
 
-    validates :token, presence: true, uniqueness: true
-    validates :refresh_token, uniqueness: true, if: :use_refresh_token?
+    validates :token, presence: true, uniqueness: { case_sensitive: true }
+    validates :refresh_token, uniqueness: { case_sensitive: true }, if: :use_refresh_token?
 
     # @attr_writer [Boolean, nil] use_refresh_token
     #   indicates the possibility of using refresh token

lib/doorkeeper/orm/active_record/application.rb

@@ -10,7 +10,7 @@ class Application < ActiveRecord::Base
     has_many :access_tokens, dependent: :delete_all, class_name: "Doorkeeper::AccessToken"
 
     validates :name, :secret, :uid, presence: true
-    validates :uid, uniqueness: true
+    validates :uid, uniqueness: { case_sensitive: true }
     validates :redirect_uri, redirect_uri: true
     validates :confidential, inclusion: { in: [true, false] }
 
@@ -60,6 +60,12 @@ def plaintext_secret
       end
     end
 
+    def to_json(options)
+      serializable_hash(except: :secret)
+        .merge(secret: plaintext_secret)
+        .to_json(options)
+    end
+
     private
 
     def generate_uid

spec/controllers/applications_controller_spec.rb

@@ -26,6 +26,10 @@ module Doorkeeper
 
         expect(json_response).to include("id", "name", "uid", "secret", "redirect_uri", "scopes")
 
+        application = Application.last
+        secret_from_response = json_response["secret"]
+        expect(application.secret_matches?(secret_from_response)).to be_truthy
+
         expect(json_response["name"]).to eq("Example")
         expect(json_response["redirect_uri"]).to eq("https://example.com")
       end
@@ -136,6 +140,72 @@ module Doorkeeper
     end
 
     context "when admin is authenticated" do
+      context "when application secrets are hashed" do
+        before do
+          allow(Doorkeeper.configuration).to receive(:application_secret_strategy).and_return(Doorkeeper::SecretStoring::Sha256Hash)
+        end
+
+        it "shows the application secret after creating a new application" do
+          expect do
+            post :create, params: {
+              doorkeeper_application: {
+                name: "Example",
+                redirect_uri: "https://example.com",
+              },
+            }
+          end.to change { Doorkeeper::Application.count }.by(1)
+
+          application = Application.last
+
+          secret_from_flash = flash[:application_secret]
+          expect(secret_from_flash).not_to be_empty
+          expect(application.secret_matches?(secret_from_flash)).to be_truthy
+          expect(response).to redirect_to(controller.main_app.oauth_application_url(application.id))
+
+          get :show, params: { id: application.id, format: :html }
+
+          # We don't know the application secret here (because its hashed) so we can not assert its text on the page
+          # Instead, we read it from the page and then check if it matches the application secret
+          code_element = %r{<code.*id="secret".*>(.*)<\/code>}.match(response.body)
+          secret_from_page = code_element[1]
+
+          expect(response.body).to have_selector("code#application_id", text: application.uid)
+          expect(response.body).to have_selector("code#secret")
+          expect(secret_from_page).not_to be_empty
+          expect(application.secret_matches?(secret_from_page)).to be_truthy
+        end
+
+        it "does not show an application secret when application did already exist" do
+          application = FactoryBot.create(:application)
+          get :show, params: { id: application.id, format: :html }
+
+          expect(response.body).to have_selector("code#application_id", text: application.uid)
+          expect(response.body).to have_selector("code#secret", text: "")
+        end
+
+        it "returns the application details in a json response" do
+          expect do
+            post :create, params: {
+              doorkeeper_application: {
+                name: "Example",
+                redirect_uri: "https://example.com",
+              }, format: :json,
+            }
+          end.to(change { Doorkeeper::Application.count })
+
+          expect(response).to be_successful
+
+          expect(json_response).to include("id", "name", "uid", "secret", "redirect_uri", "scopes")
+
+          application = Application.last
+          secret_from_response = json_response["secret"]
+          expect(application.secret_matches?(secret_from_response)).to be_truthy
+
+          expect(json_response["name"]).to eq("Example")
+          expect(json_response["redirect_uri"]).to eq("https://example.com")
+        end
+      end
+
       render_views
 
       before do
@@ -166,6 +236,14 @@ module Doorkeeper
         expect(response).to be_redirect
       end
 
+      it "shows application details" do
+        application = FactoryBot.create(:application)
+        get :show, params: { id: application.id, format: :html }
+
+        expect(response.body).to have_selector("code#application_id", text: application.uid)
+        expect(response.body).to have_selector("code#secret", text: application.plaintext_secret)
+      end
+
       it "does not allow mass assignment of uid or secret" do
         application = FactoryBot.create(:application)
         put :update, params: {

spec/validators/redirect_uri_validator_spec.rb

@@ -40,25 +40,25 @@
   it "is invalid when the uri is not a uri" do
     subject.redirect_uri = "]"
     expect(subject).not_to be_valid
-    expect(subject.errors[:redirect_uri].first).to eq("must be a valid URI.")
+    expect(subject.errors[:redirect_uri].first).to eq(I18n.t("activerecord.errors.models.doorkeeper/application.attributes.redirect_uri.invalid_uri"))
   end
 
   it "is invalid when the uri is relative" do
     subject.redirect_uri = "/abcd"
     expect(subject).not_to be_valid
-    expect(subject.errors[:redirect_uri].first).to eq("must be an absolute URI.")
+    expect(subject.errors[:redirect_uri].first).to eq(I18n.t("activerecord.errors.models.doorkeeper/application.attributes.redirect_uri.relative_uri"))
   end
 
   it "is invalid when the uri has a fragment" do
     subject.redirect_uri = "https://example.com/abcd#xyz"
     expect(subject).not_to be_valid
-    expect(subject.errors[:redirect_uri].first).to eq("cannot contain a fragment.")
+    expect(subject.errors[:redirect_uri].first).to eq(I18n.t("activerecord.errors.models.doorkeeper/application.attributes.redirect_uri.fragment_present"))
   end
 
   it "is invalid when scheme resolves to localhost (needs an explict scheme)" do
     subject.redirect_uri = "localhost:80"
     expect(subject).to be_invalid
-    expect(subject.errors[:redirect_uri].first).to eq(I18n.t('activerecord.errors.models.doorkeeper/application.attributes.redirect_uri.unspecified_scheme'))
+    expect(subject.errors[:redirect_uri].first).to eq(I18n.t("activerecord.errors.models.doorkeeper/application.attributes.redirect_uri.unspecified_scheme"))
   end
 
   it "is invalid if an ip address" do
@@ -143,7 +143,7 @@
       subject.redirect_uri = "http://example.com/callback"
       expect(subject).not_to be_valid
       error = subject.errors[:redirect_uri].first
-      expect(error).to eq("must be an HTTPS/SSL URI.")
+      expect(error).to eq(I18n.t("activerecord.errors.models.doorkeeper/application.attributes.redirect_uri.secured_uri"))
     end
   end