Fix handling of error message in authorization controller

app/controllers/api/openid_connect/authorizations_controller.rb

@@ -4,7 +4,7 @@ module Api
       rescue_from Rack::OAuth2::Server::Authorize::BadRequest do |e|
         logger.info e.backtrace[0, 10].join("\n")
         error, description = e.message.split(" :: ")
-        handle_params_error(error, description)
+        handle_params_error(error, "The request was malformed: please double check the client id and redirect uri.")
       end
 
       rescue_from OpenSSL::SSL::SSLError do |e|
@@ -56,8 +56,6 @@ module Api
         if prompt.include? "select_account"
           handle_params_error("account_selection_required",
                               "There is no support for choosing among multiple accounts")
-        elsif prompt.include? "none"
-          handle_prompt_none(prompt, auth)
         elsif prompt.include?("login") && logged_in_before?(60)
           reauthenticate
         elsif prompt.include? "consent"
@@ -105,19 +103,6 @@ module Api
         end
       end
 
-      def handle_prompt_none(prompt, auth)
-        if prompt == ["none"]
-          if auth
-            process_authorization_consent("true")
-          else
-            render_error "The Authentication Request cannot be completed without end-user interaction"
-          end
-        else
-          handle_params_error("invalid_request",
-                              "The 'none' value cannot be used with any other prompt value")
-        end
-      end
-
       def handle_start_point_response(endpoint)
         _status, header, response = endpoint.call(request.env)
         if response.redirect?
@@ -194,8 +179,7 @@ module Api
         if params[:client_id] && params[:redirect_uri]
           handle_params_error_when_client_id_and_redirect_uri_exists(error, error_description)
         else
-          flash[:error] = I18n.t("api.openid_connect.authorizations.new.bad_request")
-          redirect_to root_path
+          render_error error_description
         end
       end
 
@@ -204,9 +188,7 @@ module Api
         if app && app.redirect_uris.include?(params[:redirect_uri])
           redirect_prompt_error_display(error, error_description)
         else
-          flash[:error] = I18n.t("api.openid_connect.authorizations.new.client_id_not_found",
-                                 client_id: params[:client_id], redirect_uri: params[:redirect_uri])
-          redirect_to root_path
+          render_error "Invalid client id or redirect uri"
         end
       end
 
@@ -217,15 +199,36 @@ module Api
       end
 
       def auth_user_unless_prompt_none!
-        if params[:prompt] == "none" && !user_signed_in?
-          render_error "User must be first logged in when `prompt` is `none`"
-          # render json: {error:       "login_required",
-          #               description: "User must be first logged in when `prompt` is `none`"}
+        prompt = params[:prompt]
+        if prompt && prompt.include?("none")
+          handle_prompt_none
         else
           authenticate_user!
         end
       end
 
+      def handle_prompt_none
+        if params[:prompt] == "none"
+          if user_signed_in?
+            client_id = params[:client_id]
+            if client_id
+              auth = Api::OpenidConnect::Authorization.find_by_client_id_and_user(client_id, current_user)
+              if auth
+                process_authorization_consent("true")
+              else
+                handle_params_error("interaction_required", "User must already be authorized when `prompt` is `none`")
+              end
+            else
+              handle_params_error("bad_request", "Client ID is missing from request")
+            end
+          else
+            handle_params_error("login_required", "User must already be logged in when `prompt` is `none`")
+          end
+        else
+          handle_params_error("invalid_request", "The 'none' value cannot be used with any other prompt value")
+        end
+      end
+
       def render_error(error_description)
         @error_description = error_description
         render "api/openid_connect/error/error",

app/views/api/openid_connect/error/_error.html.haml

@@ -3,4 +3,5 @@
     .api-error.col-sm-6.col-sm-offset-3
       %h4
         %b= t("api.openid_connect.error_page.title")
-      %div= @error_description
+      %div{id: "openid_connect_error_description"}
+        = @error_description

features/desktop/oidc_auth_code_flow.feature

@@ -7,7 +7,7 @@ Feature: Access protected resources using auth code flow
     When I register a new client
     And I send a post request from that client to the code flow authorization endpoint using a invalid client id
     And I sign in as "kent@kent.kent"
-    Then I should see a flash message containing "No client with"
+    Then I should see a message containing "Invalid client id or redirect uri"
 
   Scenario: Application is denied authorization
     When I register a new client

features/desktop/oidc_implicit_flow.feature

@@ -7,7 +7,7 @@ Feature: Access protected resources using implicit flow
     When I register a new client
     And I send a post request from that client to the authorization endpoint using a invalid client id
     And I sign in as "kent@kent.kent"
-    Then I should see a flash message containing "No client with"
+    Then I should see a message containing "Invalid client id or redirect uri"
 
   Scenario: Application is denied authorization
     When I register a new client

features/step_definitions/oidc_common_steps.rb

@@ -33,3 +33,8 @@ Then /^I should receive an "([^\"]*)" error$/ do |error_message|
   user_info_json = JSON.parse(last_response.body)
   expect(user_info_json["error"]).to have_content(error_message)
 end
+
+Then(/^I should see a message containing "(.*?)"$/) do |message|
+  expect(find("#openid_connect_error_description").text).to eq(message)
+end
+

spec/controllers/api/openid_connect/authorizations_controller_spec.rb

@@ -71,8 +71,7 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do
         it "should return an bad request error" do
           post :new, redirect_uri: "http://localhost:3000/", response_type: "id_token",
                scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16)
-          expect(response).to redirect_to root_path
-          expect(flash[:error]).to include("Missing client id")
+          expect(response.body).to include("The request was malformed")
         end
       end
 
@@ -94,17 +93,15 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do
         it "should return an invalid request error" do
           post :new, client_id: client_with_multiple_redirects.client_id, response_type: "id_token",
                scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16)
-          expect(response).to redirect_to root_path
-          expect(flash[:error]).to include("Missing client id or redirect URI")
+          expect(response.body).to include("The request was malformed")
         end
       end
 
       context "when redirect URI does not match pre-registered URIs" do
-        it "should return an invalid request error" do
+        it "should return an invalid request error", focus: true do
           post :new, client_id: client.client_id, redirect_uri: "http://localhost:2000/",
                response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16)
-          expect(response).to redirect_to root_path
-          expect(flash[:error]).to include("No client")
+          expect(response.body).to include("Invalid client id or redirect uri")
         end
       end
 
@@ -128,8 +125,7 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do
         it "should return an interaction required error" do
           post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/",
                response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none"
-          expect(response.location).to match("error=interaction_required")
-          expect(response.location).to match("state=1234")
+          expect(response.body).to include("User must already be authorized when `prompt` is `none`")
         end
       end
 
@@ -141,7 +137,7 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do
         it "should return an interaction required error" do
           post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/",
                response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none"
-          expect(response.location).to match("error=login_required")
+          expect(response.body).to include("User must already be logged in when `prompt` is `none`")
         end
       end
 
@@ -150,7 +146,6 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do
           post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/",
                response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none consent"
           expect(response.location).to match("error=invalid_request")
-          expect(response.location).to match("state=1234")
         end
       end
 
@@ -167,8 +162,7 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do
         it "should return an account_selection_required error" do
           post :new, client_id: "random", redirect_uri: "http://localhost:3000/",
                response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none"
-          expect(response).to redirect_to root_path
-          expect(flash[:error]).to include("No client")
+          expect(response.body).to include("Invalid client id or redirect uri")
         end
       end
 
@@ -176,8 +170,7 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do
         it "should return an account_selection_required error" do
           post :new, client_id: client.client_id, redirect_uri: "http://randomuri:3000/",
                response_type: "id_token", scope: "openid", state: 1234, display: "page", prompt: "none"
-          expect(response).to redirect_to root_path
-          expect(flash[:error]).to include("No client")
+          expect(response.body).to include("Invalid client id or redirect uri")
         end
       end