Add a test case for the issue detected with cookie overflow

The test ensures that the ID token is not stored within the
cookies as that caused the cookie overflow.

lib/decidim/helsinki_profile/test/oidc_server.rb

@@ -118,18 +118,42 @@ def jwt(payload = {})
           )
         end
 
-        def token(payload = {})
+        def token(payload = {}, id_token_data = nil)
           # We need to issue the same kid for the JSON::JWK key as the server keys
           # that are generated through JWT::JWK. We use JSON::JWK in the specs to
           # control better the signing of the keys.
           kid = JSON::JWK.new(jwk_rsa_keys.first, kid: jwks.first.kid)
-          jwt = jwt(payload).sign(kid)
+          access_token = jwt(payload).sign(kid)
+          id_token =
+            if id_token_data.is_a?(Hash)
+              JSON::JWT.new(
+                # https://openid.net/specs/openid-connect-core-1_0.html#IDToken
+                {
+                  iss: access_token[:iss],
+                  sub: access_token[:sub],
+                  aud: access_token[:aud],
+                  exp: access_token[:exp],
+                  iat: access_token[:iat],
+                  **id_token_data
+                }
+              ).sign(kid)
+            end
 
           expires_at = 30.minutes.from_now
-          Rack::OAuth2::AccessToken::Bearer.new(
-            access_token: jwt.to_s,
-            expires_in: (expires_at - Time.now.utc).to_i
-          )
+          token_attrs = {
+            client: "client", # required by OpenIDConnect::AccessToken
+            access_token: access_token.to_s,
+            expires_in: (expires_at - Time.now.utc).to_i,
+            id_token: id_token&.to_s
+          }
+          # Use OpenIDConnect::AccessToken instead of
+          # Rack::OAuth2::AccessToken::Bearer in order to use the `id_token`
+          # attribute.
+          OpenIDConnect::AccessToken.new(**token_attrs.compact).tap do |token|
+            [:client, :raw_attributes].each do |attr|
+              token.send(:remove_instance_variable, :"@#{attr}")
+            end
+          end
         end
 
         def userinfo(authorization)

spec/controllers/decidim/helsinki_profile/omniauth_callbacks_controller_spec.rb

@@ -27,14 +27,14 @@
     {
       env: {
         "rack.session" => request.session,
-        "rack.session.options" => request.session.options,
-        "omniauth-helsinki.id_token" => id_token
+        "rack.session.options" => request.session.options
       }
     }
   end
-  let(:id_token) { "omniauth_id_token" }
   let(:omniauth_state) { request.session["omniauth.state"] }
+  let(:omniauth_nonce) { request.session["omniauth.nonce"] }
   let(:code) { SecureRandom.hex(16) }
+  let(:id_token_payload) { { nonce: omniauth_nonce, groups: ["testing"] } }
 
   before do
     profile_api.register_profile(profile)
@@ -86,7 +86,9 @@
         info = Decidim::HelsinkiProfile::SessionInfo.find_by(user: Decidim::User.last)
 
         expect(info).to be_a(Decidim::HelsinkiProfile::SessionInfo)
-        expect(info.id_token).to eq(id_token)
+
+        decoded = JSON::JWT.decode(info.id_token, :skip_verification).to_h
+        expect(decoded.symbolize_keys!).to include(id_token_payload)
       end
 
       context "when the session has a pending redirect" do
@@ -99,6 +101,24 @@
           expect(response).to redirect_to("/processes")
         end
       end
+
+      # This spec is just to ensure that the ID token is not stored inside a
+      # cookie as that would cause a cookie overflow exception
+      # (ActionDispatch::Cookies::CookieOverflow) in case it exceeds 4kB in
+      # size.
+      context "when the id token is very long" do
+        # Generate a payload of at least 4kB.
+        let(:id_token_payload) do
+          { nonce: omniauth_nonce, groups: 4.times.map { |num| (97 + num).chr * 1024 } }
+        end
+
+        it "does not cause an exception" do
+          user = Decidim::User.last
+
+          expect(user.sign_in_count).to eq(1)
+          expect(response).to redirect_to("/")
+        end
+      end
     end
 
     context "when storing the email address" do
@@ -182,9 +202,10 @@
         expect(Decidim::HelsinkiProfile::SessionInfo.find_by(id: previous_info.id)).to be_nil
 
         info = Decidim::HelsinkiProfile::SessionInfo.find_by(user: Decidim::User.last)
-
         expect(info).to be_a(Decidim::HelsinkiProfile::SessionInfo)
-        expect(info.id_token).to eq(id_token)
+
+        decoded = JSON::JWT.decode(info.id_token, :skip_verification).to_h
+        expect(decoded.symbolize_keys!).to include(id_token_payload)
       end
 
       it "redirects to the root path" do
@@ -239,7 +260,16 @@
           name: "helsinki_idp"
         )
         expect(authorization).to be_nil
-        expect(response).to redirect_to("/users/auth/helsinki/logout?id_token_hint=#{id_token}")
+
+        info = Decidim::HelsinkiProfile::SessionInfo.find_by(user: confirmed_user)
+        expect(info).to be_nil
+
+        redirect_uri = URI.parse(response.location)
+        redirect_params = Rack::Utils.parse_query(redirect_uri.query).symbolize_keys!
+
+        expect(redirect_uri.host).to eq(organization.host)
+        expect(redirect_uri.path).to eq("/users/auth/helsinki/logout")
+        expect(redirect_params).to match(id_token_hint: /\A[^\s]+\z/)
         expect(flash[:alert]).to eq(
           "Another user has already been identified using this identity. Please sign out and sign in again directly using Helsinki profile."
         )

spec/spec_helper.rb

@@ -107,10 +107,11 @@
           scope: form_data["scope"]
         }
         token_payload[:sub] = token_sub if respond_to?(:token_sub) && token_sub
+        idt_payload = id_token_payload if respond_to?(:id_token_payload)
 
         {
           headers: { "Content-Type" => "application/json" },
-          body: auth_server.token(token_payload).to_json
+          body: auth_server.token(token_payload, idt_payload).to_json
         }
       end
     end