From ad91686b62c8a006044b230e7628f99fca994867 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 9 Feb 2021 10:03:34 -0300 Subject: [PATCH 1/5] Test on Ruby 3+ with Rails 6+ And remove dupe entry in the exclude matrix. In order to get Ruby 3 working we needed to install `rexml` as part of the test dependencies, only done on the main Gemfile (Rails 6.1) and the 6.0 versions. (which are the only ones supported by Ruby 3.) Devise itself doesn't require `rexml` as it does nothing with it, but a dependency we use during tests seem to require it. I was able to track it down to omniauth-openid -> rack-openid -> ruby-openid requiring it: https://github.com/openid/ruby-openid/blob/13a88ad6442133a613d2b7d6601991a84b34630d/lib/openid/yadis/xrds.rb#L1 So while we have tests using omniauth-openid, we'll need this require in place as well. Ideally that upstream version of ruby-openid should have it, but it seems that one isn't updated in a while. --- .github/workflows/test.yml | 13 +++++++++++-- CHANGELOG.md | 2 ++ Gemfile | 1 + Gemfile.lock | 2 ++ gemfiles/Gemfile-rails-6-0 | 1 + 5 files changed, 17 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3937d5c438..0af48ddd53 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,6 +21,7 @@ jobs: - 2.5 - 2.6 - 2.7 + - 3.0 env: - DEVISE_ORM=active_record - DEVISE_ORM=mongoid @@ -59,8 +60,6 @@ jobs: gemfile: gemfiles/Gemfile-rails-4-2 - ruby: 2.7 gemfile: gemfiles/Gemfile-rails-4-1 - - ruby: 2.7 - gemfile: gemfiles/Gemfile-rails-4-1 - ruby: 2.7 gemfile: gemfiles/Gemfile-rails-4-2 - ruby: 2.7 @@ -69,6 +68,16 @@ jobs: gemfile: gemfiles/Gemfile-rails-5-1 - ruby: 2.7 gemfile: gemfiles/Gemfile-rails-5-2 + - ruby: 3.0 + gemfile: gemfiles/Gemfile-rails-4-1 + - ruby: 3.0 + gemfile: gemfiles/Gemfile-rails-4-2 + - ruby: 3.0 + gemfile: gemfiles/Gemfile-rails-5-0 + - ruby: 3.0 + gemfile: gemfiles/Gemfile-rails-5-1 + - ruby: 3.0 + gemfile: gemfiles/Gemfile-rails-5-2 - env: DEVISE_ORM=mongoid gemfile: Gemfile - env: DEVISE_ORM=mongoid diff --git a/CHANGELOG.md b/CHANGELOG.md index 995f9f088c..7cdbdd6c22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ * Devise now enables the upgrade of OmniAuth 2+. Previously Devise would raise an error if you'd try to upgrade. Please note that OmniAuth 2 is considered a security upgrade and recommended to everyone. You can read more about the details (and possible necessary changes to your app as part of the upgrade) in [their release notes](https://github.com/omniauth/omniauth/releases/tag/v2.0.0). [Devise's OmniAuth Overview wiki](https://github.com/heartcombo/devise/wiki/OmniAuth:-Overview) was also updated to cover OmniAuth 2.0 requirements. - Note that the upgrade required Devise shared links that initiate the OmniAuth flow to be changed to `method: :post`, which is now a requirement for OmniAuth, part of the security improvement. If you have copied and customized the Devise shared links partial to your app, or if you have other links in your app that initiate the OmniAuth flow, they will have to be updated to use `method: :post`, or changed to use buttons (e.g. `button_to`) to work with OmniAuth 2. (if you're using links with `method: :post`, make sure your app has `rails-ujs` or `jquery-ujs` included in order for these links to work properly.) - As part of the OmniAuth 2.0 upgrade you might also need to add the [`omniauth-rails_csrf_protection`](https://github.com/cookpad/omniauth-rails_csrf_protection) gem to your app if you don't have it already. (and you don't want to roll your own code to verify requests.) Check the OmniAuth v2 release notes for more info. + * Add support for Ruby 3. + * Add support for Rails 6.1. * Move CI to GitHub Actions. * deprecations diff --git a/Gemfile b/Gemfile index f91123340b..22ca4afe93 100644 --- a/Gemfile +++ b/Gemfile @@ -18,6 +18,7 @@ gem "responders", "~> 3.0" group :test do gem "omniauth-facebook" gem "omniauth-openid" + gem "rexml" gem "timecop" gem "webrat", "0.7.3", require: false gem "mocha", "~> 1.1", require: false diff --git a/Gemfile.lock b/Gemfile.lock index c178bd18f8..e08446d7bf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -183,6 +183,7 @@ GEM responders (3.0.1) actionpack (>= 5.0) railties (>= 5.0) + rexml (3.2.4) ruby-openid (2.9.2) ruby2_keywords (0.0.4) sprockets (4.0.2) @@ -223,6 +224,7 @@ DEPENDENCIES rails-controller-testing! rdoc responders (~> 3.0) + rexml sqlite3 (~> 1.4) timecop webrat (= 0.7.3) diff --git a/gemfiles/Gemfile-rails-6-0 b/gemfiles/Gemfile-rails-6-0 index e43284853a..d01a464fd7 100644 --- a/gemfiles/Gemfile-rails-6-0 +++ b/gemfiles/Gemfile-rails-6-0 @@ -16,6 +16,7 @@ gem "responders", "~> 3.0" group :test do gem "omniauth-facebook" gem "omniauth-openid" + gem "rexml" gem "timecop" gem "webrat", "0.7.3", require: false gem "mocha", "~> 1.1", require: false From a793472a3e28e8b0dec137531e3de64d91ff81ec Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Wed, 10 Feb 2021 17:17:29 -0300 Subject: [PATCH 2/5] Replace XML with JSON serialization across the test suite This allows us to remove the dependency on the XML serializer provided by the external `activemodel-serializers-xml` gem, and eliminates the following deprecation warning: DEPRECATION WARNING: ActiveModel::Errors#to_xml is deprecated and will be removed in Rails 6.2. Please note: this does not mean Devise doesn't support XML, it simply means our test suite will use JSON to test non-navigatable formats instead of XML, for simplicity. Devise's job is not to test object serialization, so as long as your objects properly serialize to XML/JSON/any other format, it should work out of the box. --- Gemfile | 2 - Gemfile.lock | 10 ---- gemfiles/Gemfile-rails-5-0 | 2 - gemfiles/Gemfile-rails-5-1 | 2 - gemfiles/Gemfile-rails-5-2 | 2 - gemfiles/Gemfile-rails-6-0 | 2 - test/failure_app_test.rb | 10 ++-- test/integration/authenticatable_test.rb | 33 ++++--------- test/integration/confirmable_test.rb | 32 +++++-------- test/integration/http_authenticatable_test.rb | 23 +++++----- test/integration/lockable_test.rb | 31 +++++-------- test/integration/recoverable_test.rb | 46 ++++++++----------- test/integration/registerable_test.rb | 37 ++++++--------- test/models/serializable_test.rb | 15 ------ test/rails_app/app/active_record/user.rb | 1 - .../app/controllers/users_controller.rb | 3 +- test/routes_test.rb | 12 ++--- test/test/controller_helpers_test.rb | 6 +-- 18 files changed, 93 insertions(+), 176 deletions(-) diff --git a/Gemfile b/Gemfile index 22ca4afe93..2bca340602 100644 --- a/Gemfile +++ b/Gemfile @@ -9,8 +9,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing", github: "rails/rails-controller-testing" gem "responders", "~> 3.0" diff --git a/Gemfile.lock b/Gemfile.lock index e08446d7bf..e19528ac0c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,12 +1,3 @@ -GIT - remote: https://github.com/rails/activemodel-serializers-xml.git - revision: 694f4071c6b16e4c8597cc323c241b5f787b3ea8 - specs: - activemodel-serializers-xml (1.0.2) - activemodel (>= 5.0.0.a) - activesupport (>= 5.0.0.a) - builder (~> 3.1) - GIT remote: https://github.com/rails/rails-controller-testing.git revision: 4b15c86e82ee380f2a7cc009e470368f7520560a @@ -213,7 +204,6 @@ PLATFORMS ruby DEPENDENCIES - activemodel-serializers-xml! devise! mocha (~> 1.1) omniauth diff --git a/gemfiles/Gemfile-rails-5-0 b/gemfiles/Gemfile-rails-5-0 index dcd1ac14ef..2f60c3a2b3 100644 --- a/gemfiles/Gemfile-rails-5-0 +++ b/gemfiles/Gemfile-rails-5-0 @@ -9,8 +9,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing" gem "responders", "~> 2.1" diff --git a/gemfiles/Gemfile-rails-5-1 b/gemfiles/Gemfile-rails-5-1 index c2b8f523f8..c566e9c84f 100644 --- a/gemfiles/Gemfile-rails-5-1 +++ b/gemfiles/Gemfile-rails-5-1 @@ -7,8 +7,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing" gem "responders", "~> 2.1" diff --git a/gemfiles/Gemfile-rails-5-2 b/gemfiles/Gemfile-rails-5-2 index dbfbd5f6c0..5dc267def1 100644 --- a/gemfiles/Gemfile-rails-5-2 +++ b/gemfiles/Gemfile-rails-5-2 @@ -7,8 +7,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing" gem "responders", "~> 2.1" diff --git a/gemfiles/Gemfile-rails-6-0 b/gemfiles/Gemfile-rails-6-0 index d01a464fd7..f840fc8d7a 100644 --- a/gemfiles/Gemfile-rails-6-0 +++ b/gemfiles/Gemfile-rails-6-0 @@ -7,8 +7,6 @@ gem "omniauth" gem "omniauth-oauth2" gem "rdoc" -gem "activemodel-serializers-xml", github: "rails/activemodel-serializers-xml" - gem "rails-controller-testing", github: "rails/rails-controller-testing" gem "responders", "~> 3.0" diff --git a/test/failure_app_test.rb b/test/failure_app_test.rb index 1b0aeb04aa..809f668de4 100644 --- a/test/failure_app_test.rb +++ b/test/failure_app_test.rb @@ -220,8 +220,8 @@ def call_failure(env_params = {}) end test 'works for any navigational format' do - swap Devise, navigational_formats: [:xml] do - call_failure('formats' => Mime[:xml]) + swap Devise, navigational_formats: [:json] do + call_failure('formats' => Mime[:json]) assert_equal 302, @response.first end end @@ -236,7 +236,7 @@ def call_failure(env_params = {}) context 'For HTTP request' do test 'return 401 status' do - call_failure('formats' => Mime[:xml]) + call_failure('formats' => Mime[:json]) assert_equal 401, @response.first end @@ -258,13 +258,13 @@ def call_failure(env_params = {}) end test 'return WWW-authenticate headers if model allows' do - call_failure('formats' => Mime[:xml]) + call_failure('formats' => Mime[:json]) assert_equal 'Basic realm="Application"', @response.second["WWW-Authenticate"] end test 'does not return WWW-authenticate headers if model does not allow' do swap Devise, http_authenticatable: false do - call_failure('formats' => Mime[:xml]) + call_failure('formats' => Mime[:json]) assert_nil @response.second["WWW-Authenticate"] end end diff --git a/test/integration/authenticatable_test.rb b/test/integration/authenticatable_test.rb index fcc1d734b6..fbe1da6cc0 100644 --- a/test/integration/authenticatable_test.rb +++ b/test/integration/authenticatable_test.rb @@ -462,14 +462,6 @@ class AuthenticationOthersTest < Devise::IntegrationTest end end - test 'sign in stub in xml format' do - get new_user_session_path(format: 'xml') - assert_match '', response.body - assert_match %r{.*}m, response.body - assert_match '', response.body - assert_match '\n) + assert_includes response.body, '{"user":{' end - test 'sign in with xml format is idempotent' do - get new_user_session_path(format: 'xml') + test 'sign in with json format is idempotent' do + get new_user_session_path(format: 'json') assert_response :success create_user - post user_session_path(format: 'xml'), params: { user: {email: "user@test.com", password: '12345678'} } + post user_session_path(format: 'json'), params: { user: {email: "user@test.com", password: '12345678'} } assert_response :success - get new_user_session_path(format: 'xml') + get new_user_session_path(format: 'json') assert_response :success - post user_session_path(format: 'xml'), params: { user: {email: "user@test.com", password: '12345678'} } + post user_session_path(format: 'json'), params: { user: {email: "user@test.com", password: '12345678'} } assert_response :success - assert_includes response.body, %(\n) + assert_includes response.body, '{"user":{' end test 'sign out with html redirects' do @@ -527,13 +519,6 @@ class AuthenticationOthersTest < Devise::IntegrationTest assert_current_url '/' end - test 'sign out with xml format returns no content' do - sign_in_as_user - delete destroy_user_session_path(format: 'xml') - assert_response :no_content - refute warden.authenticated?(:user) - end - test 'sign out with json format returns no content' do sign_in_as_user delete destroy_user_session_path(format: 'json') diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb index 165954617b..278f9488eb 100644 --- a/test/integration/confirmable_test.rb +++ b/test/integration/confirmable_test.rb @@ -214,40 +214,32 @@ def resend_confirmation end end - test 'resent confirmation token with valid E-Mail in XML format should return valid response' do + test 'resent confirmation token with valid e-mail in JSON format should return empty and valid response' do user = create_user(confirm: false) - post user_confirmation_path(format: 'xml'), params: { user: { email: user.email } } + post user_confirmation_path(format: 'json'), params: { user: { email: user.email } } assert_response :success - assert_equal({}.to_xml, response.body) + assert_equal({}.to_json, response.body) end - test 'resent confirmation token with invalid E-Mail in XML format should return invalid response' do + test 'resent confirmation token with invalid e-mail in JSON format should return invalid response' do create_user(confirm: false) - post user_confirmation_path(format: 'xml'), params: { user: { email: 'invalid.test@test.com' } } + post user_confirmation_path(format: 'json'), params: { user: { email: 'invalid.test@test.com' } } assert_response :unprocessable_entity - assert_includes response.body, %(\n) + assert_includes response.body, '{"errors":{' end - test 'confirm account with valid confirmation token in XML format should return valid response' do + test 'confirm account with valid confirmation token in JSON format should return valid response' do user = create_user(confirm: false) - get user_confirmation_path(confirmation_token: user.raw_confirmation_token, format: 'xml') + get user_confirmation_path(confirmation_token: user.raw_confirmation_token, format: 'json') assert_response :success - assert_includes response.body, %(\n) + assert_includes response.body, '{"user":{' end - test 'confirm account with invalid confirmation token in XML format should return invalid response' do + test 'confirm account with invalid confirmation token in JSON format should return invalid response' do create_user(confirm: false) - get user_confirmation_path(confirmation_token: 'invalid_confirmation', format: 'xml') + get user_confirmation_path(confirmation_token: 'invalid_confirmation', format: 'json') assert_response :unprocessable_entity - assert_includes response.body, %(\n) - end - - test 'request an account confirmation account with JSON, should return an empty JSON' do - user = create_user(confirm: false) - - post user_confirmation_path, params: { user: { email: user.email }, format: :json } - assert_response :success - assert_equal({}.to_json, response.body) + assert_includes response.body, '{"confirmation_token":[' end test "when in paranoid mode and with a valid e-mail, should not say that the e-mail is valid" do diff --git a/test/integration/http_authenticatable_test.rb b/test/integration/http_authenticatable_test.rb index 619a3cd821..6832159578 100644 --- a/test/integration/http_authenticatable_test.rb +++ b/test/integration/http_authenticatable_test.rb @@ -22,10 +22,10 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, skip_session_storage: [] do sign_in_as_new_user_with_http assert_response 200 - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) - get users_path(format: :xml) + get users_path(format: :json) assert_response 200 end end @@ -34,10 +34,10 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, skip_session_storage: [:http_auth] do sign_in_as_new_user_with_http assert_response 200 - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) - get users_path(format: :xml) + get users_path(format: :json) assert_response 401 end end @@ -51,8 +51,8 @@ class HttpAuthenticationTest < Devise::IntegrationTest test 'uses the request format as response content type' do sign_in_as_new_user_with_http("unknown") assert_equal 401, status - assert_equal "application/xml; charset=utf-8", headers["Content-Type"] - assert_match "Invalid Email or password.", response.body + assert_equal "application/json; charset=utf-8", headers["Content-Type"] + assert_match '"error":"Invalid Email or password."', response.body end test 'returns a custom response with www-authenticate and chosen realm' do @@ -67,7 +67,7 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, authentication_keys: [:username] do sign_in_as_new_user_with_http("usertest") assert_response :success - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) end end @@ -76,7 +76,7 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, authentication_keys: { username: false, email: false } do sign_in_as_new_user_with_http("usertest") assert_response :success - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) end end @@ -85,7 +85,7 @@ class HttpAuthenticationTest < Devise::IntegrationTest swap Devise, authentication_keys: { email: false, username: false }, http_authentication_key: :username do sign_in_as_new_user_with_http("usertest") assert_response :success - assert_match 'user@test.com', response.body + assert_match '"email":"user@test.com"', response.body assert warden.authenticated?(:user) end end @@ -101,14 +101,13 @@ class HttpAuthenticationTest < Devise::IntegrationTest private def sign_in_as_new_user_with_http(username = "user@test.com", password = "12345678") user = create_user - get users_path(format: :xml), headers: { "HTTP_AUTHORIZATION" => "Basic #{Base64.encode64("#{username}:#{password}")}" } + get users_path(format: :json), headers: { "HTTP_AUTHORIZATION" => "Basic #{Base64.encode64("#{username}:#{password}")}" } user end # Sign in with oauth2 token. This is just to test that it isn't misinterpreted as basic authentication def add_oauth2_header user = create_user - get users_path(format: :xml), headers: { "HTTP_AUTHORIZATION" => "OAuth #{Base64.encode64("#{user.email}:12345678")}" } + get users_path(format: :json), headers: { "HTTP_AUTHORIZATION" => "OAuth #{Base64.encode64("#{user.email}:12345678")}" } end - end diff --git a/test/integration/lockable_test.rb b/test/integration/lockable_test.rb index b0eaf02f57..437d8eec98 100644 --- a/test/integration/lockable_test.rb +++ b/test/integration/lockable_test.rb @@ -130,46 +130,39 @@ def send_unlock_request end end - test 'user should be able to request a new unlock token via XML request' do + test 'user should be able to request a new unlock token via JSON request and should return empty and valid response' do user = create_user(locked: true) ActionMailer::Base.deliveries.clear - post user_unlock_path(format: 'xml'), params: { user: {email: user.email} } + post user_unlock_path(format: 'json'), params: { user: {email: user.email} } assert_response :success - assert_equal({}.to_xml, response.body) + assert_equal({}.to_json, response.body) assert_equal 1, ActionMailer::Base.deliveries.size end - test 'unlocked user should not be able to request a unlock token via XML request' do + test 'unlocked user should not be able to request a unlock token via JSON request' do user = create_user(locked: false) ActionMailer::Base.deliveries.clear - post user_unlock_path(format: 'xml'), params: { user: {email: user.email} } + post user_unlock_path(format: 'json'), params: { user: {email: user.email} } assert_response :unprocessable_entity - assert_includes response.body, %(\n) + assert_includes response.body, '{"errors":{' assert_equal 0, ActionMailer::Base.deliveries.size end - test 'user with valid unlock token should be able to unlock account via XML request' do + test 'user with valid unlock token should be able to unlock account via JSON request' do user = create_user() raw = user.lock_access! assert user.access_locked? - get user_unlock_path(format: 'xml', unlock_token: raw) + get user_unlock_path(format: 'json', unlock_token: raw) assert_response :success - assert_includes response.body, %(\n) + assert_includes response.body, '{"user":{' end - test 'user with invalid unlock token should not be able to unlock the account via XML request' do - get user_unlock_path(format: 'xml', unlock_token: 'invalid_token') + test 'user with invalid unlock token should not be able to unlock the account via JSON request' do + get user_unlock_path(format: 'json', unlock_token: 'invalid_token') assert_response :unprocessable_entity - assert_includes response.body, %(\n) - end - - test "when using json to ask a unlock request, should not return the user" do - user = create_user(locked: true) - post user_unlock_path(format: "json", user: {email: user.email}) - assert_response :success - assert_equal({}.to_json, response.body) + assert_includes response.body, '{"unlock_token":[' end test "in paranoid mode, when trying to unlock a user that exists it should not say that it exists if it is locked" do diff --git a/test/integration/recoverable_test.rb b/test/integration/recoverable_test.rb index 2f1ca6e927..7626607816 100644 --- a/test/integration/recoverable_test.rb +++ b/test/integration/recoverable_test.rb @@ -261,63 +261,53 @@ def reset_password(options = {}, &block) end end - test 'reset password request with valid E-Mail in XML format should return valid response' do + test 'reset password request with valid e-mail in JSON format should return empty and valid response' do create_user - post user_password_path(format: 'xml'), params: { user: {email: "user@test.com"} } + post user_password_path(format: 'json'), params: { user: {email: "user@test.com"} } assert_response :success - assert_equal({}.to_xml, response.body) + assert_equal({}.to_json, response.body) end - test 'reset password request with invalid E-Mail in XML format should return valid response' do + test 'reset password request with invalid e-mail in JSON format should return valid response' do create_user - post user_password_path(format: 'xml'), params: { user: {email: "invalid.test@test.com"} } + post user_password_path(format: 'json'), params: { user: {email: "invalid.test@test.com"} } assert_response :unprocessable_entity - assert_includes response.body, %(\n) + assert_includes response.body, '{"errors":{' end - test 'reset password request with invalid E-Mail in XML format should return empty and valid response' do + test 'reset password request with invalid e-mail in JSON format should return empty and valid response in paranoid mode' do swap Devise, paranoid: true do create_user - post user_password_path(format: 'xml'), params: { user: {email: "invalid@test.com"} } + post user_password_path(format: 'json'), params: { user: {email: "invalid@test.com"} } assert_response :success - assert_equal({}.to_xml, response.body) + assert_equal({}.to_json, response.body) end end - test 'change password with valid parameters in XML format should return valid response' do + test 'change password with valid parameters in JSON format should return valid response' do create_user request_forgot_password - put user_password_path(format: 'xml'), params: { user: { + put user_password_path(format: 'json'), params: { user: { reset_password_token: 'abcdef', password: '987654321', password_confirmation: '987654321' - } - } + } } assert_response :success assert warden.authenticated?(:user) end - test 'change password with invalid token in XML format should return invalid response' do + test 'change password with invalid token in JSON format should return invalid response' do create_user request_forgot_password - put user_password_path(format: 'xml'), params: { user: {reset_password_token: 'invalid.token', password: '987654321', password_confirmation: '987654321'} } + put user_password_path(format: 'json'), params: { user: {reset_password_token: 'invalid.token', password: '987654321', password_confirmation: '987654321'} } assert_response :unprocessable_entity - assert_includes response.body, %(\n) + assert_includes response.body, '{"errors":{' end - test 'change password with invalid new password in XML format should return invalid response' do + test 'change password with invalid new password in JSON format should return invalid response' do user = create_user request_forgot_password - put user_password_path(format: 'xml'), params: { user: {reset_password_token: user.reload.reset_password_token, password: '', password_confirmation: '987654321'} } + put user_password_path(format: 'json'), params: { user: {reset_password_token: user.reload.reset_password_token, password: '', password_confirmation: '987654321'} } assert_response :unprocessable_entity - assert_includes response.body, %(\n) - end - - test "when using json requests to ask a confirmable request, should not return the object" do - user = create_user(confirm: false) - - post user_password_path(format: :json), params: { user: { email: user.email } } - - assert_response :success - assert_equal "{}", response.body + assert_includes response.body, '{"errors":{' end test "when in paranoid mode and with an invalid e-mail, asking to reset a password should display a message that does not indicates that the e-mail does not exists in the database" do diff --git a/test/integration/registerable_test.rb b/test/integration/registerable_test.rb index fa2610edf8..b407223312 100644 --- a/test/integration/registerable_test.rb +++ b/test/integration/registerable_test.rb @@ -283,13 +283,6 @@ def user_sign_up assert_redirected_to new_user_registration_path end - test 'a user with XML sign up stub' do - get new_user_registration_path(format: 'xml') - assert_response :success - assert_match %(\n), response.body - assert_no_match(/\n) + assert_includes response.body, '{"admin":{' admin = Admin.to_adapter.find_first(order: [:id, :desc]) assert_equal 'new_user@test.com', admin.email end - test 'a user sign up with valid information in XML format should return valid response' do - post user_registration_path(format: 'xml'), params: { user: { email: 'new_user@test.com', password: 'new_user123', password_confirmation: 'new_user123' } } + test 'a user sign up with valid information in JSON format should return valid response' do + post user_registration_path(format: 'json'), params: { user: { email: 'new_user@test.com', password: 'new_user123', password_confirmation: 'new_user123' } } assert_response :success - assert_includes response.body, %(\n) + assert_includes response.body, '{"user":{' user = User.to_adapter.find_first(order: [:id, :desc]) assert_equal 'new_user@test.com', user.email end - test 'a user sign up with invalid information in XML format should return invalid response' do - post user_registration_path(format: 'xml'), params: { user: { email: 'new_user@test.com', password: 'new_user123', password_confirmation: 'invalid' } } + test 'a user sign up with invalid information in JSON format should return invalid response' do + post user_registration_path(format: 'json'), params: { user: { email: 'new_user@test.com', password: 'new_user123', password_confirmation: 'invalid' } } assert_response :unprocessable_entity - assert_includes response.body, %(\n) + assert_includes response.body, '{"errors":{' end - test 'a user update information with valid data in XML format should return valid response' do + test 'a user update information with valid data in JSON format should return valid response' do user = sign_in_as_user - put user_registration_path(format: 'xml'), params: { user: { current_password: '12345678', email: 'user.new@test.com' } } + put user_registration_path(format: 'json'), params: { user: { current_password: '12345678', email: 'user.new@test.com' } } assert_response :success assert_equal 'user.new@test.com', user.reload.email end - test 'a user update information with invalid data in XML format should return invalid response' do + test 'a user update information with invalid data in JSON format should return invalid response' do user = sign_in_as_user - put user_registration_path(format: 'xml'), params: { user: { current_password: 'invalid', email: 'user.new@test.com' } } + put user_registration_path(format: 'json'), params: { user: { current_password: 'invalid', email: 'user.new@test.com' } } assert_response :unprocessable_entity assert_equal 'user@test.com', user.reload.email end - test 'a user cancel their account in XML format should return valid response' do + test 'a user cancel their account in JSON format should return valid response' do sign_in_as_user - delete user_registration_path(format: 'xml') + delete user_registration_path(format: 'json') assert_response :success assert_equal 0, User.to_adapter.find_all.size end diff --git a/test/models/serializable_test.rb b/test/models/serializable_test.rb index 602cbe3714..53f0f59f43 100644 --- a/test/models/serializable_test.rb +++ b/test/models/serializable_test.rb @@ -7,21 +7,6 @@ class SerializableTest < ActiveSupport::TestCase @user = create_user end - test 'should not include unsafe keys on XML' do - assert_match(/email/, @user.to_xml) - assert_no_match(/confirmation-token/, @user.to_xml) - end - - test 'should not include unsafe keys on XML even if a new except is provided' do - assert_no_match(/email/, @user.to_xml(except: :email)) - assert_no_match(/confirmation-token/, @user.to_xml(except: :email)) - end - - test 'should include unsafe keys on XML if a force_except is provided' do - assert_no_match(/ Date: Mon, 15 Feb 2021 14:45:04 -0300 Subject: [PATCH 3/5] Bundle update --- Gemfile.lock | 118 +++++++++++++++++++++++++-------------------------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index e19528ac0c..48b9a7b4e0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -20,60 +20,60 @@ PATH GEM remote: https://rubygems.org/ specs: - actioncable (6.1.1) - actionpack (= 6.1.1) - activesupport (= 6.1.1) + actioncable (6.1.2.1) + actionpack (= 6.1.2.1) + activesupport (= 6.1.2.1) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.1.1) - actionpack (= 6.1.1) - activejob (= 6.1.1) - activerecord (= 6.1.1) - activestorage (= 6.1.1) - activesupport (= 6.1.1) + actionmailbox (6.1.2.1) + actionpack (= 6.1.2.1) + activejob (= 6.1.2.1) + activerecord (= 6.1.2.1) + activestorage (= 6.1.2.1) + activesupport (= 6.1.2.1) mail (>= 2.7.1) - actionmailer (6.1.1) - actionpack (= 6.1.1) - actionview (= 6.1.1) - activejob (= 6.1.1) - activesupport (= 6.1.1) + actionmailer (6.1.2.1) + actionpack (= 6.1.2.1) + actionview (= 6.1.2.1) + activejob (= 6.1.2.1) + activesupport (= 6.1.2.1) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.1.1) - actionview (= 6.1.1) - activesupport (= 6.1.1) + actionpack (6.1.2.1) + actionview (= 6.1.2.1) + activesupport (= 6.1.2.1) rack (~> 2.0, >= 2.0.9) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.1.1) - actionpack (= 6.1.1) - activerecord (= 6.1.1) - activestorage (= 6.1.1) - activesupport (= 6.1.1) + actiontext (6.1.2.1) + actionpack (= 6.1.2.1) + activerecord (= 6.1.2.1) + activestorage (= 6.1.2.1) + activesupport (= 6.1.2.1) nokogiri (>= 1.8.5) - actionview (6.1.1) - activesupport (= 6.1.1) + actionview (6.1.2.1) + activesupport (= 6.1.2.1) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (6.1.1) - activesupport (= 6.1.1) + activejob (6.1.2.1) + activesupport (= 6.1.2.1) globalid (>= 0.3.6) - activemodel (6.1.1) - activesupport (= 6.1.1) - activerecord (6.1.1) - activemodel (= 6.1.1) - activesupport (= 6.1.1) - activestorage (6.1.1) - actionpack (= 6.1.1) - activejob (= 6.1.1) - activerecord (= 6.1.1) - activesupport (= 6.1.1) + activemodel (6.1.2.1) + activesupport (= 6.1.2.1) + activerecord (6.1.2.1) + activemodel (= 6.1.2.1) + activesupport (= 6.1.2.1) + activestorage (6.1.2.1) + actionpack (= 6.1.2.1) + activejob (= 6.1.2.1) + activerecord (= 6.1.2.1) + activesupport (= 6.1.2.1) marcel (~> 0.3.1) mimemagic (~> 0.3.2) - activesupport (6.1.1) + activesupport (6.1.2.1) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -81,7 +81,7 @@ GEM zeitwerk (~> 2.3) bcrypt (3.1.16) builder (3.2.4) - concurrent-ruby (1.1.7) + concurrent-ruby (1.1.8) crass (1.0.6) erubi (1.10.0) faraday (1.3.0) @@ -92,7 +92,7 @@ GEM globalid (0.4.2) activesupport (>= 4.2.0) hashie (4.1.0) - i18n (1.8.7) + i18n (1.8.9) concurrent-ruby (~> 1.0) jwt (2.2.2) loofah (2.9.0) @@ -111,7 +111,7 @@ GEM multi_json (1.15.0) multi_xml (0.6.0) multipart-post (2.1.1) - nio4r (2.5.4) + nio4r (2.5.5) nokogiri (1.11.1) mini_portile2 (~> 2.5.0) racc (~> 1.4) @@ -121,7 +121,7 @@ GEM multi_json (~> 1.3) multi_xml (~> 0.5) rack (>= 1.2, < 3) - omniauth (2.0.1) + omniauth (2.0.2) hashie (>= 3.4.6) rack (>= 1.6.2, < 3) rack-protection @@ -143,29 +143,29 @@ GEM rack rack-test (1.1.0) rack (>= 1.0, < 3) - rails (6.1.1) - actioncable (= 6.1.1) - actionmailbox (= 6.1.1) - actionmailer (= 6.1.1) - actionpack (= 6.1.1) - actiontext (= 6.1.1) - actionview (= 6.1.1) - activejob (= 6.1.1) - activemodel (= 6.1.1) - activerecord (= 6.1.1) - activestorage (= 6.1.1) - activesupport (= 6.1.1) + rails (6.1.2.1) + actioncable (= 6.1.2.1) + actionmailbox (= 6.1.2.1) + actionmailer (= 6.1.2.1) + actionpack (= 6.1.2.1) + actiontext (= 6.1.2.1) + actionview (= 6.1.2.1) + activejob (= 6.1.2.1) + activemodel (= 6.1.2.1) + activerecord (= 6.1.2.1) + activestorage (= 6.1.2.1) + activesupport (= 6.1.2.1) bundler (>= 1.15.0) - railties (= 6.1.1) + railties (= 6.1.2.1) sprockets-rails (>= 2.0.0) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) rails-html-sanitizer (1.3.0) loofah (~> 2.3) - railties (6.1.1) - actionpack (= 6.1.1) - activesupport (= 6.1.1) + railties (6.1.2.1) + actionpack (= 6.1.2.1) + activesupport (= 6.1.2.1) method_source rake (>= 0.8.7) thor (~> 1.0) @@ -185,8 +185,8 @@ GEM activesupport (>= 4.0) sprockets (>= 3.0.0) sqlite3 (1.4.2) - thor (1.0.1) - timecop (0.9.2) + thor (1.1.0) + timecop (0.9.4) tzinfo (2.0.4) concurrent-ruby (~> 1.0) warden (1.2.9) From faef12cf2b620e7a454138aa7c2ec1e1719e4025 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Mon, 15 Feb 2021 16:07:38 -0300 Subject: [PATCH 4/5] Use the 6-0-stable version of Rails to fix issue with JSON responses The test suite was failing on Rails 6.0 + Ruby 3 with errors like: Expected "{\"errors\":\"#\"}" to include "{\"errors\":{". The ActiveModel::Errors object wasn't being serialized to JSON as expected, and this only happened with that combination of Ruby/Rails. Upon further investigation, this was caused by a change in Ruby and fixed in Rails in this PR: https://github.com/rails/rails/pull/39697 (which describes in more details the exact same problem and links to the Ruby bug tracker with more information). That fix was backported to 6-0-stable in June 2020, but hasn't been officially released in a stable version yet: (there have been only security fixes since then for 6.0) https://github.com/rails/rails/commit/75f6539d0e94c76d93d61feef06c3b0974fe62c1 Since the branch contains the fix, I'm pointing directly to it to get the tests passing. We can't tell if there'll be a new stable 6.0 release at this point, but hopefully yes, in which case we can go back at pointing to it. --- gemfiles/Gemfile-rails-6-0 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gemfiles/Gemfile-rails-6-0 b/gemfiles/Gemfile-rails-6-0 index f840fc8d7a..bc9c83d008 100644 --- a/gemfiles/Gemfile-rails-6-0 +++ b/gemfiles/Gemfile-rails-6-0 @@ -2,7 +2,7 @@ source "https://rubygems.org" gemspec path: ".." -gem "rails", '~> 6.0.0' +gem "rails", '~> 6.0.0', github: 'rails/rails', branch: '6-0-stable' gem "omniauth" gem "omniauth-oauth2" gem "rdoc" From 1ba53dc3695508da709de547af92831e09374090 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Mon, 15 Feb 2021 16:23:47 -0300 Subject: [PATCH 5/5] Lock bundler to 2.2.9 instead of latest 2.2.10 is causing the dependency resolution on Rails 6-0-stable to fail: ``` Bundler could not find compatible versions for gem "railties": In Gemfile-rails-6-0: devise was resolved to 4.7.3, which depends on railties (>= 4.1.0) rails was resolved to 6.0.3.5, which depends on railties (= 6.0.3.5) responders (~> 3.0) was resolved to 3.0.1, which depends on railties (>= 5.0) Took 27.49 seconds ``` https://github.com/heartcombo/devise/runs/1905780158?check_suite_focus=true#step:5:23 The `railties` version 6.0.3.5 should work, given the other two are using >= declarations, but it fails in 2.2.10. Downgrading to 2.2.9 works. --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0af48ddd53..efe63d78dd 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -100,7 +100,7 @@ jobs: with: ruby-version: ${{ matrix.ruby }} bundler-cache: true # runs bundle install and caches installed gems automatically - bundler: ${{ env.BUNDLER_VERSION || 'latest' }} + bundler: ${{ env.BUNDLER_VERSION || '2.2.9' }} - uses: supercharge/mongodb-github-action@1.3.0 if: ${{ matrix.env == 'DEVISE_ORM=mongoid' }} - run: bundle exec rake