Add nonce validation for implicit OIDC flow

Per OpenID Connect Core 1.0 Section 3.2.2.1, nonce is REQUIRED for
implicit flow (id_token and id_token token response types). Add server-
side validation to enforce this requirement.

- Add validate_nonce method to PreAuthorization module
- Register validation using Doorkeeper's validation DSL
- Update existing tests to include nonce for implicit flow requests
- Add new tests for nonce validation behavior

Agent-Logs-Url: https://github.com/doorkeeper-gem/doorkeeper-openid_connect/sessions/7792b5af-9397-4ed2-9520-787df9698a07

Co-authored-by: nbulaj <1443426+nbulaj@users.noreply.github.com>

Author: Copilot

.gitignore

@@ -1,4 +1,5 @@
 /.bundle
+/gemfiles/.bundle
 /vendor/bundle
 /Gemfile.lock
 /spec/dummy/db/*.sqlite3*

gemfiles/.bundle/config

@@ -4,6 +4,10 @@ module Doorkeeper
   module OpenidConnect
     module OAuth
       module PreAuthorization
+        def self.prepended(base)
+          base.validate :nonce, error: Doorkeeper::Errors::InvalidRequest
+        end
+
         attr_reader :nonce
 
         def initialize(server, attrs = {}, resource_owner = nil)
@@ -22,6 +26,26 @@ def response_on_fragment?
 
           grant_flow&.default_response_mode == 'fragment'
         end
+
+        private
+
+        # Per OpenID Connect Core 1.0 Section 3.2.2.1, nonce is REQUIRED for the
+        # implicit flow (id_token and id_token token response types).
+        def validate_nonce
+          return true unless openid_implicit_flow?
+
+          if nonce.blank?
+            @missing_param = :nonce
+            return false
+          end
+
+          true
+        end
+
+        def openid_implicit_flow?
+          scopes.include?('openid') &&
+            response_type.to_s.split(' ').include?('id_token')
+        end
       end
     end
   end

lib/doorkeeper/openid_connect/oauth/pre_authorization.rb

@@ -139,15 +139,15 @@ def expect_successful_callback!
           it 'redirect as the fragment style uri when response_type is implicit flow request' do
             allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])
 
-            authorize! request_param.merge(response_type: 'id_token token')
+            authorize! request_param.merge(response_type: 'id_token token', nonce: 'abc123')
 
             expect(response).to redirect_to build_redirect_uri(error_params, type: 'fragment')
           end
 
           it 'set @authorize_response variable and render form_post template and when the form_post response_mode is specified' do
             allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])
 
-            authorize! request_param.merge(response_type: 'id_token token', response_mode: 'form_post')
+            authorize! request_param.merge(response_type: 'id_token token', response_mode: 'form_post', nonce: 'abc123')
 
             authorize_response = controller.instance_variable_get :@authorize_response
             expect(authorize_response.body.to_json).to eq(error_params.to_json)
@@ -174,15 +174,15 @@ def expect_successful_callback!
           it 'redirect as the fragment style uri when response_type is implicit flow request' do
             allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])
 
-            authorize! request_param.merge(response_type: 'id_token token', prompt: 'none', state: 'somestate')
+            authorize! request_param.merge(response_type: 'id_token token', prompt: 'none', state: 'somestate', nonce: 'abc123')
 
             expect(response).to redirect_to build_redirect_uri(error_params, type: 'fragment')
           end
 
           it 'set @authorize_response variable and render form_post template and when the form_post response_mode is specified' do
             allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])
 
-            authorize! request_param.merge(response_type: 'id_token token', response_mode: 'form_post', prompt: 'none', state: 'somestate')
+            authorize! request_param.merge(response_type: 'id_token token', response_mode: 'form_post', prompt: 'none', state: 'somestate', nonce: 'abc123')
 
             authorize_response = controller.instance_variable_get :@authorize_response
             expect(authorize_response.body.to_json).to eq(error_params.to_json)
@@ -218,15 +218,15 @@ def expect_successful_callback!
           it 'uses the fragment style uris when redirecting an error for implicit flow request' do
             allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])
 
-            authorize! request_param.merge(response_type: 'id_token token', prompt: 'none', state: 'somestate')
+            authorize! request_param.merge(response_type: 'id_token token', prompt: 'none', state: 'somestate', nonce: 'abc123')
 
             expect(response).to redirect_to build_redirect_uri(error_params, type: 'fragment')
           end
 
           it 'set @authorize_response variable and render form_post template and when the form_post response_mode is specified' do
             allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])
 
-            authorize! request_param.merge(response_type: 'id_token token', response_mode: 'form_post', prompt: 'none', state: 'somestate')
+            authorize! request_param.merge(response_type: 'id_token token', response_mode: 'form_post', prompt: 'none', state: 'somestate', nonce: 'abc123')
 
             authorize_response = controller.instance_variable_get :@authorize_response
             expect(authorize_response.body.to_json).to eq(error_params.to_json)
@@ -520,4 +520,38 @@ def select_account!
       expect(assigns(:pre_auth).nonce).to eq '123456'
     end
   end
+
+  describe 'nonce validation' do
+    context 'with implicit flow (id_token response type)' do
+      before do
+        allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])
+      end
+
+      it 'returns an error when nonce is missing' do
+        get :new, params: {
+          response_type: 'id_token',
+          current_user: user.id,
+          client_id: application.uid,
+          scope: 'openid',
+          redirect_uri: application.redirect_uri,
+        }
+
+        expect(response).to have_http_status(:bad_request)
+        expect(response).to render_template('doorkeeper/authorizations/error')
+      end
+
+      it 'succeeds when nonce is present' do
+        get :new, params: {
+          response_type: 'id_token',
+          current_user: user.id,
+          client_id: application.uid,
+          scope: 'openid',
+          redirect_uri: application.redirect_uri,
+          nonce: 'abc123',
+        }
+
+        expect(response).to render_template('doorkeeper/authorizations/new')
+      end
+    end
+  end
 end

spec/controllers/doorkeeper/authorizations_controller_spec.rb

@@ -6,6 +6,7 @@
   subject { Doorkeeper::OAuth::PreAuthorization.new server, attrs }
 
   let(:server) { Doorkeeper.configuration }
+  let(:application) { create :application, scopes: 'openid public' }
   let(:attrs) {}
 
   describe '#initialize' do
@@ -18,6 +19,100 @@
     end
   end
 
+  describe '#authorizable?' do
+    let(:client) { Doorkeeper::OAuth::Client.new(application) }
+
+    context 'with response_type = id_token (implicit flow)' do
+      let(:base_attrs) do
+        {
+          client_id: client.uid,
+          response_type: 'id_token',
+          redirect_uri: 'https://app.com/callback',
+          scope: 'openid',
+        }
+      end
+
+      before do
+        allow(server).to receive(:grant_flows).and_return(['implicit_oidc'])
+      end
+
+      it 'is not authorizable without a nonce' do
+        pre_auth = Doorkeeper::OAuth::PreAuthorization.new(server, base_attrs)
+        pre_auth.authorizable?
+
+        expect(pre_auth).not_to be_authorizable
+      end
+
+      it 'is authorizable with a nonce' do
+        pre_auth = Doorkeeper::OAuth::PreAuthorization.new(server, base_attrs.merge(nonce: 'abc123'))
+        pre_auth.authorizable?
+
+        expect(pre_auth).to be_authorizable
+      end
+
+      it 'sets missing_param to :nonce when nonce is absent' do
+        pre_auth = Doorkeeper::OAuth::PreAuthorization.new(server, base_attrs)
+        pre_auth.authorizable?
+
+        expect(pre_auth.missing_param).to eq :nonce
+      end
+    end
+
+    context 'with response_type = id_token token (implicit flow)' do
+      let(:base_attrs) do
+        {
+          client_id: client.uid,
+          response_type: 'id_token token',
+          redirect_uri: 'https://app.com/callback',
+          scope: 'openid',
+        }
+      end
+
+      before do
+        allow(server).to receive(:grant_flows).and_return(['implicit_oidc'])
+      end
+
+      it 'is not authorizable without a nonce' do
+        pre_auth = Doorkeeper::OAuth::PreAuthorization.new(server, base_attrs)
+        pre_auth.authorizable?
+
+        expect(pre_auth).not_to be_authorizable
+      end
+
+      it 'is authorizable with a nonce' do
+        pre_auth = Doorkeeper::OAuth::PreAuthorization.new(server, base_attrs.merge(nonce: 'abc123'))
+        pre_auth.authorizable?
+
+        expect(pre_auth).to be_authorizable
+      end
+    end
+
+    context 'with response_type = code (authorization code flow)' do
+      let(:base_attrs) do
+        {
+          client_id: client.uid,
+          response_type: 'code',
+          redirect_uri: 'https://app.com/callback',
+          scope: 'openid',
+        }
+      end
+
+      it 'is authorizable without a nonce' do
+        pre_auth = Doorkeeper::OAuth::PreAuthorization.new(server, base_attrs)
+        pre_auth.authorizable?
+
+        expect(pre_auth).to be_authorizable
+      end
+
+      it 'is authorizable with a nonce' do
+        pre_auth = Doorkeeper::OAuth::PreAuthorization.new(server, base_attrs.merge(nonce: 'abc123'))
+        pre_auth.authorizable?
+
+        expect(pre_auth).to be_authorizable
+      end
+    end
+  end
+
   describe '#error_response' do
     context 'with response_type = code' do
       let(:attrs) { { response_type: 'code', redirect_uri: 'client.com/callback' } }