Skip to content

Commit

Permalink
DEV: Make tests more resilient (discourse#16279)
Browse files Browse the repository at this point in the history
Since we give a 200 response for login errors, we should be checking
whether the error key exists in each case or not.

Some tests were broken, because they weren't checking.
  • Loading branch information
danielwaterworth authored Mar 25, 2022
1 parent 136f7db commit 9ce6280
Showing 1 changed file with 41 additions and 0 deletions.
41 changes: 41 additions & 0 deletions spec/requests/session_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
user.update(admin: true)
get "/session/email-login/#{email_token.token}.json"
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
end
end

Expand All @@ -46,6 +47,7 @@
user.update(admin: true)
get "/session/email-login/#{email_token.token}.json"
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
end
end

Expand Down Expand Up @@ -134,6 +136,7 @@
user.update(admin: true)
post "/session/email-login/#{email_token.token}.json"
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(session[:current_user_id]).to eq(user.id)
end
end
Expand Down Expand Up @@ -257,6 +260,7 @@
it "sets the user_option timezone for the user" do
post "/session/email-login/#{email_token.token}.json", params: { timezone: "Australia/Melbourne" }
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(user.reload.user_option.timezone).to eq("Australia/Melbourne")
end
end
Expand Down Expand Up @@ -421,6 +425,7 @@
}

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
user.reload

expect(session[:current_user_id]).to eq(user.id)
Expand Down Expand Up @@ -1344,12 +1349,14 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
context 'local login via email is disabled' do
before do
SiteSetting.enable_local_logins_via_email = false
EmailToken.confirm(email_token.token)
end
it 'doesnt matter, logs in correctly' do
post "/session.json", params: {
login: user.username, password: 'myawesomepassword'
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
end
end

Expand Down Expand Up @@ -1446,6 +1453,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
end

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(events.map { |event| event[:event_name] }).to contain_exactly(
:user_logged_in, :user_first_logged_in
)
Expand All @@ -1464,6 +1472,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
login: user.username, password: 'myawesomepassword', timezone: "Australia/Melbourne"
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(user.reload.user_option.timezone).to eq("Australia/Melbourne")
end
end
Expand Down Expand Up @@ -1544,6 +1553,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
}

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
user.reload

expect(session[:current_user_id]).to eq(user.id)
Expand Down Expand Up @@ -1638,6 +1648,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
second_factor_method: UserSecondFactor.methods[:totp]
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
user.reload

expect(session[:current_user_id]).to eq(user.id)
Expand All @@ -1658,6 +1669,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
second_factor_method: UserSecondFactor.methods[:backup_codes]
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
user.reload

expect(session[:current_user_id]).to eq(user.id)
Expand All @@ -1680,6 +1692,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
login: "@" + user.username, password: 'myawesomepassword'
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).to be_present
user.reload

expect(session[:current_user_id]).to be_nil
Expand All @@ -1692,6 +1705,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
login: "@" + user.username, password: 'myawesomepassword'
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
user.reload

expect(session[:current_user_id]).to eq(user.id)
Expand All @@ -1704,6 +1718,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
login: user.email, password: 'myawesomepassword'
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(session[:current_user_id]).to eq(user.id)
end
end
Expand Down Expand Up @@ -1744,6 +1759,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)

it "doesn't log in the user" do
expect(response.status).to eq(200)
expect(response.parsed_body['error']).to be_present
expect(session[:current_user_id]).to be_blank
end

Expand All @@ -1764,6 +1780,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
login: user.email, password: 'myawesomepassword'
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(session[:current_user_id]).to eq(user.id)
end
end
Expand All @@ -1786,6 +1803,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
login: user.username, password: 'myawesomepassword'
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(session[:current_user_id]).to eq(user.id)
end

Expand Down Expand Up @@ -1813,6 +1831,7 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
}

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(session[:current_user_id]).to eq(user.id)
end
end
Expand All @@ -1828,6 +1847,7 @@ def post_login
it "doesn't log in the user" do
post_login
expect(response.status).to eq(200)
expect(response.parsed_body['error']).to be_present
expect(session[:current_user_id]).to be_blank
end

Expand Down Expand Up @@ -1857,13 +1877,15 @@ def post_login
SiteSetting.max_logins_per_ip_per_hour = 2
RateLimiter.enable
RateLimiter.clear_all!
EmailToken.confirm(email_token.token)

2.times do
post "/session.json", params: {
login: user.username, password: 'myawesomepassword'
}

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
end

post "/session.json", params: {
Expand All @@ -1887,6 +1909,7 @@ def post_login
second_factor_method: UserSecondFactor.methods[:totp]
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).to be_present
end

post "/session.json", params: {
Expand All @@ -1904,6 +1927,7 @@ def post_login
it 'rate limits second factor attempts by login' do
RateLimiter.enable
RateLimiter.clear_all!
EmailToken.confirm(email_token.token)

6.times do |x|
post "/session.json", params: {
Expand All @@ -1914,6 +1938,7 @@ def post_login
}, env: { "REMOTE_ADDR": "1.2.3.#{x}" }

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
end

[user.username + " ", user.username.capitalize, user.username].each_with_index do |username , x|
Expand Down Expand Up @@ -1947,6 +1972,7 @@ def post_login
delete "/session/#{user.username}.json", xhr: true

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(session[:current_user_id]).to be_blank
expect(response.cookies["_t"]).to be_blank

Expand All @@ -1960,12 +1986,14 @@ def post_login
user = sign_in(Fabricate(:user))
delete "/session/#{user.username}.json", xhr: true
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(response.parsed_body["redirect_url"]).to eq("/")

SiteSetting.login_required = true
user = sign_in(Fabricate(:user))
delete "/session/#{user.username}.json", xhr: true
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(response.parsed_body["redirect_url"]).to eq("/login")
end

Expand All @@ -1980,6 +2008,7 @@ def post_login
delete "/session/#{user.username}.json", xhr: true

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(response.parsed_body["redirect_url"]).to eq("/myredirect/#{user.username}")
ensure
DiscourseEvent.off(:before_session_destroy, &callback)
Expand Down Expand Up @@ -2013,6 +2042,7 @@ def post_login
get "/session/otp/#{token}"

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(response.body).to include(
I18n.t("user_api_key.otp_confirmation.logging_in_as", username: user.username)
)
Expand Down Expand Up @@ -2050,6 +2080,7 @@ def post_login

get "/session/current.json"
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
end
end
end
Expand Down Expand Up @@ -2078,6 +2109,7 @@ def post_login
params: { login: user.username }

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(Jobs::CriticalUserEmail.jobs.size).to eq(1)
end

Expand All @@ -2086,6 +2118,7 @@ def post_login
params: { login: user.email }

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(Jobs::CriticalUserEmail.jobs.size).to eq(1)
end
end
Expand Down Expand Up @@ -2120,6 +2153,7 @@ def post_login
3.times do
post "/session/forgot_password.json", params: { login: user.username }
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
end

post "/session/forgot_password.json", params: { login: user.username }
Expand All @@ -2131,6 +2165,7 @@ def post_login
headers: { 'REMOTE_ADDR' => '10.1.1.1' }

expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
end

post "/session/forgot_password.json",
Expand Down Expand Up @@ -2253,6 +2288,7 @@ def post_login
it "returns the JSON for the user" do
get "/session/current.json"
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
json = response.parsed_body
expect(json['current_user']).to be_present
expect(json['current_user']['id']).to eq(user.id)
Expand Down Expand Up @@ -2285,6 +2321,7 @@ def post_login
nonce = response.parsed_body["second_factor_challenge_nonce"]
get "/session/2fa.json", params: { nonce: nonce }
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present

freeze_time (SecondFactor::AuthManager::MAX_CHALLENGE_AGE + 1.minute).from_now
get "/session/2fa.json", params: { nonce: nonce }
Expand All @@ -2297,6 +2334,7 @@ def post_login
nonce = response.parsed_body["second_factor_challenge_nonce"]
get "/session/2fa.json", params: { nonce: nonce }
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
challenge_data = response.parsed_body
expect(challenge_data["totp_enabled"]).to eq(true)
expect(challenge_data["backup_enabled"]).to eq(false)
Expand All @@ -2318,6 +2356,7 @@ def post_login
nonce = response.parsed_body["second_factor_challenge_nonce"]
get "/session/2fa.json", params: { nonce: nonce }
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
challenge_data = response.parsed_body
expect(challenge_data["totp_enabled"]).to eq(true)
expect(challenge_data["backup_enabled"]).to eq(true)
Expand Down Expand Up @@ -2390,13 +2429,15 @@ def post_login
second_factor_token: token
}
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(response.parsed_body["ok"]).to eq(true)
expect(response.parsed_body["callback_method"]).to eq("POST")
expect(response.parsed_body["callback_path"]).to eq("/session/2fa/test-action")
expect(response.parsed_body["redirect_path"]).to eq("/ggg")

post "/session/2fa/test-action", params: { second_factor_nonce: nonce }
expect(response.status).to eq(200)
expect(response.parsed_body['error']).not_to be_present
expect(response.parsed_body["result"]).to eq("second_factor_auth_completed")
end

Expand Down

0 comments on commit 9ce6280

Please sign in to comment.