Skip to content

Commit

Permalink
Replace XML with JSON serialization across the test suite
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
carlosantoniodasilva committed Feb 15, 2021
1 parent ad91686 commit a793472
Show file tree
Hide file tree
Showing 18 changed files with 93 additions and 176 deletions.
2 changes: 0 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 0 additions & 10 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -213,7 +204,6 @@ PLATFORMS
ruby

DEPENDENCIES
activemodel-serializers-xml!
devise!
mocha (~> 1.1)
omniauth
Expand Down
2 changes: 0 additions & 2 deletions gemfiles/Gemfile-rails-5-0
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions gemfiles/Gemfile-rails-5-1
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions gemfiles/Gemfile-rails-5-2
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions gemfiles/Gemfile-rails-6-0
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 5 additions & 5 deletions test/failure_app_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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
Expand Down
33 changes: 9 additions & 24 deletions test/integration/authenticatable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 '<?xml version="1.0" encoding="UTF-8"?>', response.body
assert_match %r{<user>.*</user>}m, response.body
assert_match '<email></email>', response.body
assert_match '<password nil="true"', response.body
end

test 'sign in stub in json format' do
get new_user_session_path(format: 'json')
assert_match '{"user":{', response.body
Expand All @@ -492,27 +484,27 @@ class AuthenticationOthersTest < Devise::IntegrationTest
refute warden.authenticated?(:admin)
end

test 'sign in with xml format returns xml response' do
test 'sign in with json format returns json response' do
create_user
post user_session_path(format: 'xml'), params: { user: {email: "[email protected]", password: '12345678'} }
post user_session_path(format: 'json'), params: { user: {email: "[email protected]", password: '12345678'} }
assert_response :success
assert_includes response.body, %(<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<user>)
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: "[email protected]", password: '12345678'} }
post user_session_path(format: 'json'), params: { user: {email: "[email protected]", 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: "[email protected]", password: '12345678'} }
post user_session_path(format: 'json'), params: { user: {email: "[email protected]", password: '12345678'} }
assert_response :success
assert_includes response.body, %(<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<user>)
assert_includes response.body, '{"user":{'
end

test 'sign out with html redirects' do
Expand All @@ -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')
Expand Down
32 changes: 12 additions & 20 deletions test/integration/confirmable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: '[email protected]' } }
post user_confirmation_path(format: 'json'), params: { user: { email: '[email protected]' } }
assert_response :unprocessable_entity
assert_includes response.body, %(<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<errors>)
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, %(<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<user>)
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, %(<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<errors>)
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
Expand Down
23 changes: 11 additions & 12 deletions test/integration/http_authenticatable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 '<email>[email protected]</email>', response.body
assert_match '"email":"[email protected]"', response.body
assert warden.authenticated?(:user)

get users_path(format: :xml)
get users_path(format: :json)
assert_response 200
end
end
Expand All @@ -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 '<email>[email protected]</email>', response.body
assert_match '"email":"[email protected]"', response.body
assert warden.authenticated?(:user)

get users_path(format: :xml)
get users_path(format: :json)
assert_response 401
end
end
Expand All @@ -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 "<error>Invalid Email or password.</error>", 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
Expand All @@ -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 '<email>[email protected]</email>', response.body
assert_match '"email":"[email protected]"', response.body
assert warden.authenticated?(:user)
end
end
Expand All @@ -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 '<email>[email protected]</email>', response.body
assert_match '"email":"[email protected]"', response.body
assert warden.authenticated?(:user)
end
end
Expand All @@ -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 '<email>[email protected]</email>', response.body
assert_match '"email":"[email protected]"', response.body
assert warden.authenticated?(:user)
end
end
Expand All @@ -101,14 +101,13 @@ class HttpAuthenticationTest < Devise::IntegrationTest
private
def sign_in_as_new_user_with_http(username = "[email protected]", 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
31 changes: 12 additions & 19 deletions test/integration/lockable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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, %(<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<errors>)
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, %(<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<user>)
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, %(<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<errors>)
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
Expand Down
Loading

0 comments on commit a793472

Please sign in to comment.