add :unauthorized_client error to unredirectable and status is unauho…

…rized

lib/doorkeeper/oauth/error_response.rb

@@ -32,15 +32,17 @@ def body
       end
 
       def status
-        if name == :invalid_client
+        if name == :invalid_client || name == :unauthorized_client
           :unauthorized
         else
           :bad_request
         end
       end
 
       def redirectable?
-        name != :invalid_redirect_uri && name != :invalid_client &&
+        name != :invalid_redirect_uri &&
+          name != :invalid_client &&
+          name != :unauthorized_client &&
           !URIChecker.oob_uri?(@redirect_uri)
       end
 

lib/doorkeeper/oauth/pre_authorization.rb

@@ -84,6 +84,11 @@ def validate_client_supports_grant_flow
         Doorkeeper.config.allow_grant_flow_for_client?(grant_type, client.application)
       end
 
+      def validate_resource_owner_authorize_for_client
+        # The `authorize_resource_owner_for_client` config option is used for this validation
+        client.application.authorized_for_resource_owner?(@resource_owner)
+      end
+
       def validate_redirect_uri
         return false if redirect_uri.blank?
 
@@ -125,11 +130,6 @@ def validate_code_challenge_method
           (code_challenge_method.present? && code_challenge_method =~ /^plain$|^S256$/)
       end
 
-      def validate_resource_owner_authorize_for_client
-        # The `authorize_resource_owner_for_client` config option is used for this validation
-        client.application.authorized_for_resource_owner?(@resource_owner)
-      end
-
       def response_on_fragment?
         response_type == "token"
       end

spec/controllers/authorizations_controller_spec.rb

@@ -144,6 +144,37 @@ def query_params
       end
     end
 
+    context "when client can not use grant flow" do
+      before do
+        config_is_set(:allow_grant_flow_for_client, ->(*_) { false })
+        post :create, params: {
+          client_id: client.uid,
+          response_type: "token",
+          redirect_uri: client.redirect_uri,
+        }
+      end
+
+      let(:response_json_body) { JSON.parse(response.body) }
+
+      it "renders 401 error" do
+        expect(response.status).to eq 401
+      end
+
+      it "includes error name" do
+        expect(response_json_body["error"]).to eq("unauthorized_client")
+      end
+
+      it "includes error description" do
+        expect(response_json_body["error_description"]).to eq(
+          translated_error_message(:unauthorized_client),
+        )
+      end
+
+      it "does not issue any access token" do
+        expect(Doorkeeper::AccessToken.all).to be_empty
+      end
+    end
+
     context "when user cannot access application" do
       before do
         allow(Doorkeeper.configuration).to receive(:authorize_resource_owner_for_client).and_return(->(*_) { false })
@@ -156,7 +187,7 @@ def query_params
 
       let(:response_json_body) { JSON.parse(response.body) }
 
-      it "renders 400 error" do
+      it "renders 401 error" do
         expect(response.status).to eq 401
       end
 
@@ -214,10 +245,10 @@ def query_params
   end
 
   describe "POST #create in API mode with errors" do
+    before { config_is_set(:api_only, true) }
+
     context "when missing client_id" do
       before do
-        allow(Doorkeeper.config).to receive(:api_only).and_return(true)
-
         post :create, params: {
           client_id: "",
           response_type: "token",
@@ -248,7 +279,37 @@ def query_params
 
     context "when user cannot access application" do
       before do
-        allow(Doorkeeper.configuration).to receive(:api_only).and_return(true)
+        config_is_set(:allow_grant_flow_for_client, ->(*_) { false })
+        post :create, params: {
+          client_id: client.uid,
+          response_type: "token",
+          redirect_uri: client.redirect_uri,
+        }
+      end
+
+      let(:response_json_body) { JSON.parse(response.body) }
+
+      it "renders 401 error" do
+        expect(response.status).to eq 401
+      end
+
+      it "includes error name" do
+        expect(response_json_body["error"]).to eq("unauthorized_client")
+      end
+
+      it "includes error description" do
+        expect(response_json_body["error_description"]).to eq(
+          translated_error_message(:unauthorized_client),
+        )
+      end
+
+      it "does not issue any access token" do
+        expect(Doorkeeper::AccessToken.all).to be_empty
+      end
+    end
+
+    context "when user cannot access application" do
+      before do
         allow(Doorkeeper.configuration).to receive(:authorize_resource_owner_for_client).and_return(->(*_) { false })
 
         post :create, params: {
@@ -260,7 +321,7 @@ def query_params
 
       let(:response_json_body) { JSON.parse(response.body) }
 
-      it "renders 400 error" do
+      it "renders 401 error" do
         expect(response.status).to eq 401
       end
 
@@ -281,7 +342,6 @@ def query_params
 
     context "when other error happens" do
       before do
-        allow(Doorkeeper.config).to receive(:api_only).and_return(true)
         default_scopes_exist :public
 
         post :create, params: {

spec/lib/oauth/error_response_spec.rb

@@ -13,6 +13,12 @@
 
       expect(subject.status).to eq(:unauthorized)
     end
+
+    it "has a status of unauthorized for an unauthorized_client error" do
+      subject = described_class.new(name: :unauthorized_client)
+
+      expect(subject.status).to eq(:unauthorized)
+    end
   end
 
   describe ".from_request" do
@@ -62,4 +68,36 @@
       it { expect(headers).to include("error_description=\"#{error_response.description}\"") }
     end
   end
+
+  describe ".redirectable?" do
+    it "not redirectable when error name is invalid_redirect_uri" do
+      subject = described_class.new(name: :invalid_redirect_uri, redirect_uri: 'https://example.com')
+
+      expect(subject.redirectable?).to be false
+    end
+
+    it "not redirectable when error name is invalid_client" do
+      subject = described_class.new(name: :invalid_client, redirect_uri: 'https://example.com')
+
+      expect(subject.redirectable?).to be false
+    end
+
+    it "not redirectable when error name is unauthorized_client" do
+      subject = described_class.new(name: :unauthorized_client, redirect_uri: 'https://example.com')
+
+      expect(subject.redirectable?).to be false
+    end
+
+    it "not redirectable when redirect_uri is oob uri" do
+      subject = described_class.new(name: :other_error, redirect_uri: Doorkeeper::OAuth::NonStandard::IETF_WG_OAUTH2_OOB)
+
+      expect(subject.redirectable?).to be false
+    end
+
+    it "is redirectable when error is not related to client or redirect_uri, and redirect_uri is not oob uri" do
+      subject = described_class.new(name: :other_error, redirect_uri: 'https://example.com')
+
+      expect(subject.redirectable?).to be true
+    end
+  end
 end

spec/lib/oauth/invalid_request_response_spec.rb

@@ -58,4 +58,18 @@
       end
     end
   end
+
+  describe ".redirectable?" do
+    it "not redirectable when missing_param is client_id" do
+      subject = described_class.new(missing_param: :client_id)
+
+      expect(subject.redirectable?).to be false
+    end
+
+    it "is redirectable when missing_param is other than client_id" do
+      subject = described_class.new(missing_param: :code_verifier)
+
+      expect(subject.redirectable?).to be true
+    end
+  end
 end

spec/lib/oauth/pre_authorization_spec.rb

@@ -27,6 +27,20 @@
     }
   end
 
+  it "must call the validations on client and redirect_uri before other validations because they are not redirectable" do
+    validation_attributes = described_class.validations.map { |validation| validation[:attribute] }
+
+    expect(validation_attributes).to eq([:client_id,
+                                         :client,
+                                         :client_supports_grant_flow,
+                                         :resource_owner_authorize_for_client,
+                                         :redirect_uri,
+                                         :params,
+                                         :response_type,
+                                         :scopes,
+                                         :code_challenge_method])
+  end
+
   it "is authorizable when request is valid" do
     expect(pre_auth).to be_authorizable
   end