Browse Source

Merge pull request #400 from communitiesuk/url-audit

URL audit
pull/399/head
Paul Robert Lloyd 3 years ago committed by GitHub
parent
commit
dd9b1b2357
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      app/controllers/auth/passwords_controller.rb
  2. 52
      config/forms/2021_2022.json
  3. 2
      config/initializers/rack_attack.rb
  4. 23
      config/routes.rb
  5. 4
      spec/features/auth/user_lockout_spec.rb
  6. 2
      spec/features/organisation_spec.rb
  7. 2
      spec/features/start_page_spec.rb
  8. 20
      spec/features/user_spec.rb
  9. 8
      spec/requests/auth/passwords_controller_spec.rb
  10. 4
      spec/requests/bulk_upload_controller_spec.rb
  11. 2
      spec/requests/case_logs_controller_spec.rb
  12. 6
      spec/requests/form_controller_spec.rb
  13. 6
      spec/requests/organisations_controller_spec.rb
  14. 6
      spec/requests/rack_attack_spec.rb
  15. 12
      spec/requests/users_controller_spec.rb

2
app/controllers/auth/passwords_controller.rb

@ -66,7 +66,7 @@ protected
end end
def after_sending_reset_password_instructions_path_for(_resource) def after_sending_reset_password_instructions_path_for(_resource)
confirmations_reset_path(email: params.dig(resource_class_name, "email")) account_password_reset_confirmation_path(email: params.dig(resource_class_name, "email"))
end end
def after_resetting_password_path_for(resource) def after_resetting_password_path_for(resource)

52
config/forms/2021_2022.json

@ -83,7 +83,7 @@
} }
} }
}, },
"startdate": { "tenancy_start_date": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -222,7 +222,7 @@
} }
} }
}, },
"do_you_know_the_local_authority": { "property_local_authority": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -611,7 +611,7 @@
} }
] ]
}, },
"unitletas": { "property_let_type": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -1027,7 +1027,7 @@
} }
] ]
}, },
"tenancy_type_starter_text": { "starter_tenancy_type": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -1071,7 +1071,7 @@
} }
] ]
}, },
"fixed_term_tenancy": { "tenancy_length": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -1145,7 +1145,7 @@
} }
], ],
"pages": { "pages": {
"number_of_members": { "household_members": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -1619,7 +1619,7 @@
} }
] ]
}, },
"person_2_relationship": { "person_2_relationship_to_lead": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -1827,7 +1827,7 @@
} }
] ]
}, },
"person_3_relationship": { "person_3_relationship_to_lead": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -2032,7 +2032,7 @@
} }
] ]
}, },
"person_4_relationship": { "person_4_relationship_to_lead": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -2234,7 +2234,7 @@
} }
] ]
}, },
"person_5_relationship": { "person_5_relationship_to_lead": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -2433,7 +2433,7 @@
} }
] ]
}, },
"person_6_relationship": { "person_6_relationship_to_lead": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -2629,7 +2629,7 @@
} }
] ]
}, },
"person_7_relationship": { "person_7_relationship_to_lead": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -2822,7 +2822,7 @@
} }
] ]
}, },
"person_8_relationship": { "person_8_relationship_to_lead": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -3033,7 +3033,7 @@
} }
} }
}, },
"armed_forces_member": { "armed_forces_serving": {
"header": "", "header": "",
"description": "", "description": "",
"depends_on": [ "depends_on": [
@ -3067,7 +3067,7 @@
} }
} }
}, },
"armed_forces_reservist": { "armed_forces_injured": {
"header": "", "header": "",
"description": "", "description": "",
"depends_on": [ "depends_on": [
@ -3101,7 +3101,7 @@
} }
} }
}, },
"pregnancy": { "pregnant": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -3188,7 +3188,7 @@
} }
} }
}, },
"condition_effects": { "health_condition_effects": {
"header": "", "header": "",
"description": "", "description": "",
"depends_on": [ "depends_on": [
@ -3253,7 +3253,7 @@
} }
], ],
"pages": { "pages": {
"time_lived_in_la": { "time_lived_in_local_authority": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -3299,7 +3299,7 @@
} }
] ]
}, },
"time_on_la_waiting_list": { "time_on_waiting_list": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -3726,7 +3726,7 @@
} }
} }
}, },
"previous_la": { "previous_local_authority": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -4490,7 +4490,7 @@
} }
], ],
"pages": { "pages": {
"net_income_known": { "income_known": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -4516,7 +4516,7 @@
} }
} }
}, },
"net_income": { "income_amount": {
"depends_on": [ "depends_on": [
{ {
"net_income_known": 0 "net_income_known": 0
@ -4576,7 +4576,7 @@
} }
} }
}, },
"net_income_value_check": { "check_income_amount": {
"depends_on": [{ "net_income_soft_validation_triggered?": true }], "depends_on": [{ "net_income_soft_validation_triggered?": true }],
"title_text": "Net income is outside the expected range based on the main tenant’s working situation", "title_text": "Net income is outside the expected range based on the main tenant’s working situation",
"informative_text": { "informative_text": {
@ -4638,7 +4638,7 @@
} }
} }
}, },
"net_income_uc_proportion": { "benefits_proportion": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -5571,7 +5571,7 @@
} }
] ]
}, },
"rent_shortfall": { "outstanding": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {
@ -5601,7 +5601,7 @@
} }
] ]
}, },
"rent_shortfall_amount": { "outstanding_amount": {
"header": "", "header": "",
"description": "", "description": "",
"questions": { "questions": {

2
config/initializers/rack_attack.rb

@ -9,7 +9,7 @@ else
end end
Rack::Attack.throttle("password reset requests", limit: 5, period: 60.seconds) do |request| Rack::Attack.throttle("password reset requests", limit: 5, period: 60.seconds) do |request|
if request.params["user"].present? && request.path == "/users/password" && request.post? if request.params["user"].present? && request.path == "/account/password" && request.post?
request.params["user"]["email"].to_s.downcase.gsub(/\s+/, "") request.params["user"]["email"].to_s.downcase.gsub(/\s+/, "")
end end
end end

23
config/routes.rb

@ -10,7 +10,11 @@ Rails.application.routes.draw do
confirmations: "active_admin/devise/confirmations", confirmations: "active_admin/devise/confirmations",
two_factor_authentication: "auth/two_factor_authentication", two_factor_authentication: "auth/two_factor_authentication",
}, },
path_names: { sign_in: "sign-in", sign_out: "sign-out", two_factor_authentication: "two-factor-authentication" }, path_names: {
sign_in: "sign-in",
sign_out: "sign-out",
two_factor_authentication: "two-factor-authentication",
},
sign_out_via: %i[get], sign_out_via: %i[get],
} }
@ -18,13 +22,20 @@ Rails.application.routes.draw do
get "admin/two-factor-authentication/resend", to: "auth/two_factor_authentication#show_resend" get "admin/two-factor-authentication/resend", to: "auth/two_factor_authentication#show_resend"
end end
devise_for :users, controllers: { devise_for :users, {
passwords: "auth/passwords", path: :account,
sessions: "auth/sessions", controllers: {
}, path_names: { sign_in: "sign-in", sign_out: "sign-out" } passwords: "auth/passwords",
sessions: "auth/sessions",
},
path_names: {
sign_in: "sign-in",
sign_out: "sign-out",
},
}
devise_scope :user do devise_scope :user do
get "confirmations/reset", to: "auth/passwords#reset_confirmation" get "account/password/reset-confirmation", to: "auth/passwords#reset_confirmation"
end end
get "/health", to: ->(_) { [204, {}, [nil]] } get "/health", to: ->(_) { [204, {}, [nil]] }

4
spec/features/auth/user_lockout_spec.rb

@ -9,7 +9,7 @@ RSpec.describe "User Lockout" do
context "when login-in with the wrong user password up to a maximum number of attempts" do context "when login-in with the wrong user password up to a maximum number of attempts" do
before do before do
visit("/users/sign-in") visit("/account/sign-in")
max_login_attempts.times do max_login_attempts.times do
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
fill_in("user[password]", with: "wrong_password") fill_in("user[password]", with: "wrong_password")
@ -18,7 +18,7 @@ RSpec.describe "User Lockout" do
end end
it "locks the user account" do it "locks the user account" do
visit("/users/sign-in") visit("/account/sign-in")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
fill_in("user[password]", with: user.password) fill_in("user[password]", with: user.password)
click_button("Sign in") click_button("Sign in")

2
spec/features/organisation_spec.rb

@ -55,7 +55,7 @@ RSpec.describe "User Features" do
name: "New User", name: "New User",
email: "new_user@example.com", email: "new_user@example.com",
organisation: organisation.name, organisation: organisation.name,
link: "http://localhost:3000/users/password/edit?reset_password_token=#{reset_password_token}", link: "http://localhost:3000/account/password/edit?reset_password_token=#{reset_password_token}",
}, },
}, },
) )

2
spec/features/start_page_spec.rb

@ -20,7 +20,7 @@ RSpec.describe "Start Page Features" do
it "takes you to sign in and then to logs" do it "takes you to sign in and then to logs" do
visit("/") visit("/")
click_link("Start now") click_link("Start now")
expect(page).to have_current_path("/users/sign-in?start=true") expect(page).to have_current_path("/account/sign-in?start=true")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
fill_in("user[password]", with: user.password) fill_in("user[password]", with: user.password)
click_button("Sign in") click_button("Sign in")

20
spec/features/user_spec.rb

@ -17,7 +17,7 @@ RSpec.describe "User Features" do
context "when the user navigates to case logs" do context "when the user navigates to case logs" do
it " is required to log in" do it " is required to log in" do
visit("/logs") visit("/logs")
expect(page).to have_current_path("/users/sign-in") expect(page).to have_current_path("/account/sign-in")
expect(page).to have_content("Sign in to your account to submit CORE data") expect(page).to have_content("Sign in to your account to submit CORE data")
end end
@ -59,11 +59,11 @@ RSpec.describe "User Features" do
it " is redirected to the reset password page when they click the reset password link" do it " is redirected to the reset password page when they click the reset password link" do
visit("/logs") visit("/logs")
click_link("reset your password") click_link("reset your password")
expect(page).to have_current_path("/users/password/new") expect(page).to have_current_path("/account/password/new")
end end
it " is shown an error message if they submit without entering an email address" do it " is shown an error message if they submit without entering an email address" do
visit("/users/password/new") visit("/account/password/new")
click_button("Send email") click_button("Send email")
expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#error-summary-title")
expect(page).to have_selector("#user-email-field-error") expect(page).to have_selector("#user-email-field-error")
@ -71,7 +71,7 @@ RSpec.describe "User Features" do
end end
it " is shown an error message if they submit an invalid email address" do it " is shown an error message if they submit an invalid email address" do
visit("/users/password/new") visit("/account/password/new")
fill_in("user[email]", with: "thisisn'tanemail") fill_in("user[email]", with: "thisisn'tanemail")
click_button("Send email") click_button("Send email")
expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#error-summary-title")
@ -80,24 +80,24 @@ RSpec.describe "User Features" do
end end
it " is redirected to check your email page after submitting an email on the reset password page" do it " is redirected to check your email page after submitting an email on the reset password page" do
visit("/users/password/new") visit("/account/password/new")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
click_button("Send email") click_button("Send email")
expect(page).to have_content("Check your email") expect(page).to have_content("Check your email")
end end
it " is shown their email on the password reset confirmation page" do it " is shown their email on the password reset confirmation page" do
visit("/users/password/new") visit("/account/password/new")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
click_button("Send email") click_button("Send email")
expect(page).to have_content(user.email) expect(page).to have_content(user.email)
end end
it " is shown the reset password confirmation page even if their email doesn't exist in the system" do it " is shown the reset password confirmation page even if their email doesn't exist in the system" do
visit("/users/password/new") visit("/account/password/new")
fill_in("user[email]", with: "idontexist@example.com") fill_in("user[email]", with: "idontexist@example.com")
click_button("Send email") click_button("Send email")
expect(page).to have_current_path("/confirmations/reset?email=idontexist%40example.com") expect(page).to have_current_path("/account/password/reset-confirmation?email=idontexist%40example.com")
end end
it " is sent a reset password email via Notify" do it " is sent a reset password email via Notify" do
@ -109,11 +109,11 @@ RSpec.describe "User Features" do
name: user.name, name: user.name,
email: user.email, email: user.email,
organisation: user.organisation.name, organisation: user.organisation.name,
link: "http://localhost:3000/users/password/edit?reset_password_token=#{reset_password_token}", link: "http://localhost:3000/account/password/edit?reset_password_token=#{reset_password_token}",
}, },
}, },
) )
visit("/users/password/new") visit("/account/password/new")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
click_button("Send email") click_button("Send email")
end end

8
spec/requests/auth/passwords_controller_spec.rb

@ -20,7 +20,7 @@ RSpec.describe Auth::PasswordsController, type: :request do
let(:email) { user.email } let(:email) { user.email }
it "redirects to the email sent page" do it "redirects to the email sent page" do
post "/users/password", params: params post "/account/password", params: params
expect(response).to have_http_status(:redirect) expect(response).to have_http_status(:redirect)
follow_redirect! follow_redirect!
expect(response.body).to match(/Check your email/) expect(response.body).to match(/Check your email/)
@ -35,7 +35,7 @@ RSpec.describe Auth::PasswordsController, type: :request do
let(:email) { "madeup_email@test.com" } let(:email) { "madeup_email@test.com" }
it "redirects to the email sent page anyway" do it "redirects to the email sent page anyway" do
post "/users/password", params: params post "/account/password", params: params
expect(response).to have_http_status(:redirect) expect(response).to have_http_status(:redirect)
follow_redirect! follow_redirect!
expect(response.body).to match(/Check your email/) expect(response.body).to match(/Check your email/)
@ -59,12 +59,12 @@ RSpec.describe Auth::PasswordsController, type: :request do
let(:message) { "Your password has been changed successfully. You are now signed in" } let(:message) { "Your password has been changed successfully. You are now signed in" }
it "changes the password" do it "changes the password" do
expect { put "/users/password", params: update_password_params } expect { put "/account/password", params: update_password_params }
.to(change { user.reload.encrypted_password }) .to(change { user.reload.encrypted_password })
end end
it "after password change, the user is signed in" do it "after password change, the user is signed in" do
put "/users/password", params: update_password_params put "/account/password", params: update_password_params
# Devise redirects once after re-sign in with new password and then root redirects as well. # Devise redirects once after re-sign in with new password and then root redirects as well.
follow_redirect! follow_redirect!
follow_redirect! follow_redirect!

4
spec/requests/bulk_upload_controller_spec.rb

@ -17,7 +17,7 @@ RSpec.describe BulkUploadController, type: :request do
before { get url, headers: headers, params: {} } before { get url, headers: headers, params: {} }
it "does not let you see the bulk upload page" do it "does not let you see the bulk upload page" do
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
@ -25,7 +25,7 @@ RSpec.describe BulkUploadController, type: :request do
before { post url, params: { bulk_upload: { case_log_bulk_upload: valid_file } } } before { post url, params: { bulk_upload: { case_log_bulk_upload: valid_file } } }
it "does not let you submit bulk uploads" do it "does not let you submit bulk uploads" do
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
end end

2
spec/requests/case_logs_controller_spec.rb

@ -200,7 +200,7 @@ RSpec.describe CaseLogsController, type: :request do
context "with a user that is not signed in" do context "with a user that is not signed in" do
it "does not let the user get case log tasklist pages they don't have access to" do it "does not let the user get case log tasklist pages they don't have access to" do
get "/logs/#{case_log.id}", headers: headers, params: {} get "/logs/#{case_log.id}", headers: headers, params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end

6
spec/requests/form_controller_spec.rb

@ -24,19 +24,19 @@ RSpec.describe FormController, type: :request do
describe "GET" do describe "GET" do
it "does not let you get case logs pages you don't have access to" do it "does not let you get case logs pages you don't have access to" do
get "/logs/#{case_log.id}/person-1-age", headers: headers, params: {} get "/logs/#{case_log.id}/person-1-age", headers: headers, params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
it "does not let you get case log check answer pages you don't have access to" do it "does not let you get case log check answer pages you don't have access to" do
get "/logs/#{case_log.id}/household-characteristics/check-answers", headers: headers, params: {} get "/logs/#{case_log.id}/household-characteristics/check-answers", headers: headers, params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
describe "POST" do describe "POST" do
it "does not let you post form answers to case logs you don't have access to" do it "does not let you post form answers to case logs you don't have access to" do
post "/logs/#{case_log.id}/form", params: {} post "/logs/#{case_log.id}/form", params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
end end

6
spec/requests/organisations_controller_spec.rb

@ -13,17 +13,17 @@ RSpec.describe OrganisationsController, type: :request do
describe "#show" do describe "#show" do
it "does not let you see organisation details from org route" do it "does not let you see organisation details from org route" do
get "/organisations/#{organisation.id}", headers: headers, params: {} get "/organisations/#{organisation.id}", headers: headers, params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
it "does not let you see organisation details from details route" do it "does not let you see organisation details from details route" do
get "/organisations/#{organisation.id}/details", headers: headers, params: {} get "/organisations/#{organisation.id}/details", headers: headers, params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
it "does not let you see organisation users" do it "does not let you see organisation users" do
get "/organisations/#{organisation.id}/users", headers: headers, params: {} get "/organisations/#{organisation.id}/users", headers: headers, params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
end end

6
spec/requests/rack_attack_spec.rb

@ -31,7 +31,7 @@ describe "Rack::Attack" do
context "when the number of requests is under the throttle limit" do context "when the number of requests is under the throttle limit" do
it "does not throttle" do it "does not throttle" do
under_limit.times do under_limit.times do
post "/users/password", params: params post "/account/password", params: params
follow_redirect! follow_redirect!
end end
last_response = response last_response = response
@ -42,7 +42,7 @@ describe "Rack::Attack" do
context "when the number of requests is at the throttle limit" do context "when the number of requests is at the throttle limit" do
it "does not throttle" do it "does not throttle" do
limit.times do limit.times do
post "/users/password", params: params post "/account/password", params: params
follow_redirect! follow_redirect!
end end
last_response = response last_response = response
@ -53,7 +53,7 @@ describe "Rack::Attack" do
context "when the number of requests is over the throttle limit" do context "when the number of requests is over the throttle limit" do
it "throttles" do it "throttles" do
over_limit.times do over_limit.times do
post "/users/password", params: params post "/account/password", params: params
follow_redirect! follow_redirect!
end end
last_response = response last_response = response

12
spec/requests/users_controller_spec.rb

@ -20,35 +20,35 @@ RSpec.describe UsersController, type: :request do
describe "#show" do describe "#show" do
it "does not let you see user details" do it "does not let you see user details" do
get "/users/#{user.id}", headers: headers, params: {} get "/users/#{user.id}", headers: headers, params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
describe "#edit" do describe "#edit" do
it "does not let you edit user details" do it "does not let you edit user details" do
get "/users/#{user.id}/edit", headers: headers, params: {} get "/users/#{user.id}/edit", headers: headers, params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
describe "#password" do describe "#password" do
it "does not let you edit user passwords" do it "does not let you edit user passwords" do
get "/users/#{user.id}/password/edit", headers: headers, params: {} get "/users/#{user.id}/password/edit", headers: headers, params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
describe "#patch" do describe "#patch" do
it "does not let you update user details" do it "does not let you update user details" do
patch "/logs/#{user.id}", params: {} patch "/logs/#{user.id}", params: {}
expect(response).to redirect_to("/users/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
describe "reset password" do describe "reset password" do
it "renders the user edit password view" do it "renders the user edit password view" do
_raw, enc = Devise.token_generator.generate(User, :reset_password_token) _raw, enc = Devise.token_generator.generate(User, :reset_password_token)
get "/users/password/edit?reset_password_token=#{enc}" get "/account/password/edit?reset_password_token=#{enc}"
expect(page).to have_css("h1", class: "govuk-heading-l", text: "Reset your password") expect(page).to have_css("h1", class: "govuk-heading-l", text: "Reset your password")
end end
@ -88,7 +88,7 @@ RSpec.describe UsersController, type: :request do
before do before do
allow(User).to receive(:find_or_initialize_with_error_by).and_return(user) allow(User).to receive(:find_or_initialize_with_error_by).and_return(user)
allow(user).to receive(:reset_password_sent_at).and_return(4.hours.ago) allow(user).to receive(:reset_password_sent_at).and_return(4.hours.ago)
put "/users/password", headers: headers, params: params put "/account/password", headers: headers, params: params
end end
it "shows an error" do it "shows an error" do

Loading…
Cancel
Save