diff --git a/.env.sample b/.env.sample index 0037c9016..d6c12f8f9 100644 --- a/.env.sample +++ b/.env.sample @@ -4,10 +4,6 @@ POSTGRES_USER=postgres REDIS_PORT=6380 REDIS_URL="redis://localhost:${REDIS_PORT}/0" BULLET=disabled -# To generate a new AUTH_SECRET just execute Digest::SHA256.digest('ftiapi or put something here') -AUTH_SECRET= -# set to true once both frontends authenticate with cookies to turn off token auth -DISABLE_BEARER_AUTH= API_KEY= SECRET_KEY_BASE= diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3ffc4b248..478a0f5e5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -9,7 +9,6 @@ env: SENDGRID_API_KEY: asdf CONTACT_EMAIL: nomail@nomail.com RESPONSIBLE_EMAIL: test@nomail.com - AUTH_SECRET: secret RAILS_ENV: test SECRET_KEY_BASE: f54c9d76c42e397e17cbc0d0a024da5a762a7a0d934839b417a77dac6fda65a49a37b32bcd229ac5fd5c1fedef8ed6acf7a57ed6465d6339862cdc0dfab8886f diff --git a/Gemfile b/Gemfile index ffb8ca429..973646a32 100644 --- a/Gemfile +++ b/Gemfile @@ -72,7 +72,6 @@ gem "globalize" # Auth and Omniauth gem "bcrypt" gem "cancancan" -gem "jwt" # Uploads gem "carrierwave-base64" diff --git a/Gemfile.lock b/Gemfile.lock index c610b4cb9..c19aa0c30 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -295,7 +295,6 @@ GEM concurrent-ruby (~> 1.1) webrick (~> 1.7) websocket-driver (~> 0.7) - ffi (1.17.4) ffi (1.17.4-aarch64-linux-gnu) ffi (1.17.4-arm64-darwin) ffi (1.17.4-x86_64-linux-gnu) @@ -524,7 +523,6 @@ GEM racc patience_diff (1.2.0) optimist (~> 3.0) - pg (1.6.3) pg (1.6.3-aarch64-linux) pg (1.6.3-arm64-darwin) pg (1.6.3-x86_64-linux) @@ -843,7 +841,6 @@ DEPENDENCIES http i18n_generators jsonapi-resources! - jwt letter_opener_web maxmind-geoip2 mini_magick diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 8bd32dbc8..774c3d3dc 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "oj" -require "auth" class APIController < ActionController::API class UnprocessableContentError < StandardError; end @@ -16,10 +15,6 @@ class UnprocessableContentError < StandardError; end # frontends allowed to namespace their own cookies and scope resources via ?app= APPS = %w[observations-tool].freeze - # kill switch for token auth once both frontends run on cookies, so the - # cutover is an .env edit and a puma restart rather than a release - DISABLE_BEARER_AUTH_ENV_VAR = "DISABLE_BEARER_AUTH" - def context {current_user: current_user, app: app_name, @@ -35,7 +30,6 @@ def context rescue_from ActiveRecord::RecordNotFound, with: :record_not_found rescue_from ActionController::RoutingError, with: :record_not_found - rescue_from JWT::VerificationError, with: :bad_auth_key rescue_from UnprocessableContentError, with: :unprocessable_content rescue_from CanCan::AccessDenied do |exception| @@ -53,7 +47,7 @@ def logged_in? def current_user @current_user ||= begin - user = user_from_bearer_token || auth_cookie_user + user = auth_cookie_user user if user&.is_active end rescue @@ -90,17 +84,8 @@ def render_unprocessable_entity_error(errors) render json: json_errors, status: :unprocessable_content end - # The Bearer JWT (API clients) takes precedence over the session cookie so an - # explicit token always wins over whatever the browser has stored. - def user_from_bearer_token - return unless bearer_token.present? - - id = Auth.decode(bearer_token)&.dig("user") - User.find_by(id: id) if id - end - - # The cookie is encrypted with the app's secret_key_base (opaque, tamper-proof) - # rather than a JWT, so its payload is not readable by the client. For + # The cookie is encrypted with the app's secret_key_base, so it is opaque and + # tamper-proof and its payload is not readable by the client. For # remember_me logins Rails embeds a server-verified expiry into the payload # via use_cookies_with_metadata; the default browser-session cookie has no # server-side expiry and is dropped client-side when the browser closes. @@ -131,20 +116,6 @@ def app_name params[:app].presence_in(APPS) end - # Pretending the header isn't there is what makes the kill switch safe: it - # disables the bearer login path and, in the same stroke, the CSRF exemption - # that keys off this method. Gating only #user_from_bearer_token would leave a - # stale Authorization header skipping CSRF on a cookie-authenticated request. - def bearer_token - return if ENV[DISABLE_BEARER_AUTH_ENV_VAR] == "true" - - request.env["HTTP_AUTHORIZATION"]&.scan(/Bearer (.*)$/)&.flatten&.last - end - - def bad_auth_key - render json: {errors: [{status: 400, title: "API Key/Authorization Key mal formed"}]}, status: :bad_request - end - def set_locale(&action) locale = if params[:locale].present? && I18n.available_locales.map { |x| x.to_s }.include?(params[:locale]) params[:locale] diff --git a/app/controllers/concerns/csrf_protection.rb b/app/controllers/concerns/csrf_protection.rb index 3193cfc80..52da8889c 100644 --- a/app/controllers/concerns/csrf_protection.rb +++ b/app/controllers/concerns/csrf_protection.rb @@ -4,11 +4,9 @@ # requests. The XSRF-TOKEN cookie is not HTTP-only so the frontend JS can # copy its value into the X-XSRF-TOKEN header on unsafe requests; a # cross-site page cannot read the cookie (same-origin policy) and so cannot -# forge the header. Bearer-authenticated clients and unauthenticated -# requests are exempt. +# forge the header. Unauthenticated requests are exempt. # -# Depends on the including controller exposing #bearer_token, #auth_cookie_user -# and #app_name. +# Depends on the including controller exposing #auth_cookie_user and #app_name. module CsrfProtection extend ActiveSupport::Concern @@ -39,7 +37,6 @@ def csrf_cookie_name def verify_csrf_token! return if request.get? || request.head? || request.options? - return if bearer_token.present? user_id = auth_cookie_user&.id return if user_id.blank? diff --git a/app/controllers/v1/sessions_controller.rb b/app/controllers/v1/sessions_controller.rb index af0050437..dcbcce96d 100644 --- a/app/controllers/v1/sessions_controller.rb +++ b/app/controllers/v1/sessions_controller.rb @@ -14,15 +14,14 @@ class SessionsController < APIController def create @user = User.find_by(email: auth_params[:email]) if @user.present? && @user.valid_password?(auth_params[:password]) && @user.is_active - token = Auth.issue({user: @user.id}) @user.update_column(:should_change_password, true) unless User.strong_password?(auth_params[:password]) @user.update_tracked_fields!(request) set_download_session_cookie_for(@user) - if ActiveModel::Type::Boolean.new.cast(auth_params[:set_cookie]) - set_auth_cookie(@user) - set_csrf_cookie(@user.id, expires: remember_me? ? REMEMBER_ME_TTL.from_now : nil) - end - render json: {token: token, role: @user.user_permission.user_role, + # cookies are the only way to authenticate now, so a login that did not + # set them would hand back a 200 and no session at all + set_auth_cookie(@user) + set_csrf_cookie(@user.id, expires: remember_me? ? REMEMBER_ME_TTL.from_now : nil) + render json: {role: @user.user_permission.user_role, user_id: @user.id, country: @user.country_id, operator_ids: @user.operator_ids, observer: @user.observer_id}, status: :ok else @@ -45,7 +44,7 @@ def download_session private def auth_params - params.expect(auth: [:email, :password, :current_sign_in_ip, :set_cookie, :remember_me]) + params.expect(auth: [:email, :password, :current_sign_in_ip, :remember_me]) end def set_auth_cookie(user) diff --git a/docker-compose.yml b/docker-compose.yml index ab31a217c..20d59a2c3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -19,7 +19,6 @@ services: - POSTGRES_HOST=db - POSTGRES_PORT=5432 - POSTGRES_USER=postgres - - AUTH_SECRET - RAILS_ENV=e2e redis: image: redis:8.0.5-alpine diff --git a/lib/auth.rb b/lib/auth.rb deleted file mode 100644 index e5d9b11e7..000000000 --- a/lib/auth.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -require "jwt" - -class Auth - ALGORITHM = "HS256" - - def self.issue(payload) - JWT.encode(payload, auth_secret, ALGORITHM) - end - - def self.decode(token) - JWT.decode(token, auth_secret, true, {algorithm: ALGORITHM}).first - rescue - nil - end - - def self.auth_secret - ENV["AUTH_SECRET"] - end -end diff --git a/spec/acceptance/countries_spec.rb b/spec/acceptance/countries_spec.rb index 30d562c87..45c35e3d8 100644 --- a/spec/acceptance/countries_spec.rb +++ b/spec/acceptance/countries_spec.rb @@ -4,11 +4,7 @@ resource "Countries" do explanation "Countries resource" - let!(:admin) { FactoryBot.create(:admin) } - let!(:admin_token) { "Bearer " + Auth.issue({user: admin.id}) } - header "Content-Type", "application/vnd.api+json" - header "Authorization", :admin_token let!(:countries) { FactoryBot.create_list(:country, 5, is_active: true) } let!(:operator) { FactoryBot.create_list(:operator, 3, country: countries.first) } diff --git a/spec/acceptance/operators_spec.rb b/spec/acceptance/operators_spec.rb index 9ed75d230..23efa204b 100644 --- a/spec/acceptance/operators_spec.rb +++ b/spec/acceptance/operators_spec.rb @@ -4,11 +4,7 @@ resource "Operators" do explanation "Operators resource" - let!(:admin) { FactoryBot.create(:admin) } - let!(:admin_token) { "Bearer " + Auth.issue({user: admin.id}) } - header "Content-Type", "application/vnd.api+json" - header "Authorization", :admin_token let!(:country) { FactoryBot.create :country } let!(:operators) { FactoryBot.create_list(:operator, 5, country: country) } diff --git a/spec/integration/v1/bearer_auth_disabling_spec.rb b/spec/integration/v1/bearer_auth_disabling_spec.rb deleted file mode 100644 index dd4407c98..000000000 --- a/spec/integration/v1/bearer_auth_disabling_spec.rb +++ /dev/null @@ -1,71 +0,0 @@ -require "rails_helper" - -module V1 - describe "Disabling bearer auth", type: :request do - def disable_bearer_auth - allow(ENV).to receive(:[]).and_call_original - allow(ENV).to receive(:[]).with(APIController::DISABLE_BEARER_AUTH_ENV_VAR).and_return("true") - end - - def login_with_cookie - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} - end - - it "accepts a bearer token while the switch is off" do - get "/users/current-user", headers: user_headers - - expect(status).to eq(200) - end - - it "rejects a bearer token once the switch is on" do - disable_bearer_auth - - get "/users/current-user", headers: user_headers - - expect(status).to eq(401) - end - - it "still authenticates with the auth cookie" do - login_with_cookie - disable_bearer_auth - - get "/users/current-user" - - expect(status).to eq(200) - expect(parsed_attributes[:email]).to eq(user.email) - end - - # the CSRF exemption keys off the Authorization header, so disabling bearer - # auth has to hide the header from that check too — otherwise a leftover - # token would wave a cookie-authenticated write straight past CSRF - it "still enforces CSRF on a cookie-authenticated request carrying a stale token" do - login_with_cookie - disable_bearer_auth - - delete "/logout", headers: {"Authorization" => "Bearer #{generate_token(user.id)}"} - - expect(status).to eq(403) - end - - it "allows the same request when the CSRF header is present" do - login_with_cookie - disable_bearer_auth - - delete "/logout", headers: { - "Authorization" => "Bearer #{generate_token(user.id)}", - APIController::CSRF_HEADER => cookies[APIController::CSRF_COOKIE_NAME] - } - - expect(status).to eq(204) - end - - it "keeps issuing a token at login so frontends reading the field do not break" do - disable_bearer_auth - - post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} - - expect(status).to eq(200) - expect(parsed_body[:token]).to be_present - end - end -end diff --git a/spec/integration/v1/laws_spec.rb b/spec/integration/v1/laws_spec.rb index 05ca18e43..2e6b50f8e 100644 --- a/spec/integration/v1/laws_spec.rb +++ b/spec/integration/v1/laws_spec.rb @@ -9,7 +9,7 @@ module V1 show: {}, create: { success_roles: %i[admin], - failure_roles: %i[operator], + failure_roles: %i[operator_user], valid_params: -> { {"min-fine": 1, "max-fine": 2, relationships: {subcategory: subcategory.id, country: country.id}} }, invalid_params: -> { {"min-fine": 1, "max-fine": -2, relationships: {subcategory: subcategory.id}} }, error_attributes: [ diff --git a/spec/integration/v1/observations_spec.rb b/spec/integration/v1/observations_spec.rb index ec63674be..774a42b83 100644 --- a/spec/integration/v1/observations_spec.rb +++ b/spec/integration/v1/observations_spec.rb @@ -16,7 +16,8 @@ module V1 let(:ngo_observer) { create(:observer) } let(:ngo) { create(:ngo, observer: ngo_observer) } - let(:ngo_headers) { authorize_headers(ngo.id) } + let(:ngo_headers) { authorize_headers(ngo.id, app: "observations-tool") } + let(:admin_tool_headers) { authorize_headers(admin.id, app: "observations-tool") } let!(:country) { create(:country) } let(:observation) { create(:observation) } @@ -165,7 +166,7 @@ module V1 it "Returns error object when the observation cannot be updated by admin" do patch("/observations/#{observation.id}?app=observations-tool", params: jsonapi_params("observations", observation.id, {"country-id": ""}), - headers: admin_headers) + headers: admin_tool_headers) expect(parsed_body).to eq(jsonapi_errors(422, 100, {relationships_country: ["must exist"]})) expect(status).to eq(422) @@ -174,7 +175,7 @@ module V1 it "Returns success object when the observation was successfully updated by admin" do patch("/observations/#{observation.id}?app=observations-tool", params: jsonapi_params("observations", observation.id, {"is-active": false}), - headers: admin_headers) + headers: admin_tool_headers) expect(parsed_attributes[:"is-active"]).to eq(false) expect(observation.reload.deactivated?).to eq(true) @@ -184,7 +185,7 @@ module V1 it "Returns success object when the observation was successfully deactivated by admin" do patch("/observations/#{observation.id}?app=observations-tool", params: jsonapi_params("observations", observation.id, {"is-active": false}), - headers: admin_headers) + headers: admin_tool_headers) expect(observation.reload.is_active).to eq(false) expect(status).to eq(200) @@ -193,7 +194,7 @@ module V1 xit "Allows to translate observation" do patch("/observations/#{observation.id}?locale=fr&app=observations-tool", params: jsonapi_params("observations", observation.id, {details: "FR Observation one"}), - headers: admin_headers) + headers: admin_tool_headers) expect(observation.reload.details).to eq("FR Observation one") I18n.with_locale(:en) do @@ -231,7 +232,7 @@ module V1 it "Status goes from Created to Ready for QC2" do patch("/observations/#{observation.id}?app=observations-tool", params: jsonapi_params("observations", observation.id, {"validation-status": "Ready for QC2"}), - headers: admin_headers) + headers: admin_tool_headers) expect(status).to eq(200) expect(parsed_body[:data][:attributes][:"validation-status"]).to eq("Ready for QC2") end @@ -239,7 +240,7 @@ module V1 it "Status cannot go to Needs revision" do patch("/observations/#{observation.id}?app=observations-tool", params: jsonapi_params("observations", observation.id, {"validation-status": "Needs revision"}), - headers: admin_headers) + headers: admin_tool_headers) expect(parsed_body[:errors].first[:title]).to eq("Invalid validation change for monitor. Can't move from 'Created' to 'Needs revision'") expect(status).to eq(422) diff --git a/spec/integration/v1/sessions_spec.rb b/spec/integration/v1/sessions_spec.rb index 8f2e83be2..73b5292d0 100644 --- a/spec/integration/v1/sessions_spec.rb +++ b/spec/integration/v1/sessions_spec.rb @@ -14,7 +14,6 @@ module V1 expect(status).to eq(200) expect(parsed_body).to eq({ - token: JWT.encode({user: user.id}, ENV["AUTH_SECRET"], "HS256"), role: "user", user_id: user.id, country: nil, operator_ids: [], observer: nil @@ -34,25 +33,26 @@ module V1 end describe "Auth cookie" do - it "does not set auth cookie by default" do + it "sets the auth cookie on every login" do post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} expect(status).to eq(200) - expect(response.cookies[APIController::AUTH_COOKIE_NAME]).to be_nil + expect(response.cookies[APIController::AUTH_COOKIE_NAME]).to be_present end - it "sets an opaque auth cookie when set_cookie param is true" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + it "sets an opaque auth cookie" do + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} expect(status).to eq(200) cookie = response.cookies[APIController::AUTH_COOKIE_NAME] expect(cookie).to be_present - # the cookie is encrypted, not a raw JWT - expect(cookie).not_to eq(JWT.encode({user: user.id}, ENV["AUTH_SECRET"], "HS256")) + # opaque to the client: the payload is encrypted, so the salt it carries + # is not readable in the cookie value + expect(cookie).not_to include(user.authenticatable_salt) end it "sets a separate auth cookie per app" do - post "/login?app=observations-tool", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login?app=observations-tool", params: {auth: {email: user.email, password: "Supersecret1"}} expect(status).to eq(200) expect(response.cookies[APIController::AUTH_COOKIE_NAME]).to be_nil @@ -60,7 +60,7 @@ module V1 end it "sets a session cookie (no expiry) by default" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} expect(status).to eq(200) set_cookie = auth_set_cookie_header @@ -70,14 +70,14 @@ module V1 end it "sets a persistent cookie when remember_me is true" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true, remember_me: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1", remember_me: true}} expect(status).to eq(200) expect(auth_set_cookie_header).to match(/expires=/i) end it "authenticates a request using the auth cookie" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} get "/users/current-user" @@ -86,7 +86,7 @@ module V1 end it "authenticates a request using the app-namespaced auth cookie" do - post "/login?app=observations-tool", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login?app=observations-tool", params: {auth: {email: user.email, password: "Supersecret1"}} get "/users/current-user?app=observations-tool" @@ -95,7 +95,7 @@ module V1 end it "ignores an unknown app and falls back to the portal cookie" do - post "/login?app=bogus", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login?app=bogus", params: {auth: {email: user.email, password: "Supersecret1"}} expect(status).to eq(200) expect(response.cookies["bogus_#{APIController::AUTH_COOKIE_NAME}"]).to be_nil @@ -103,7 +103,7 @@ module V1 end it "stops authenticating once the password changes" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} get "/users/current-user" expect(status).to eq(200) @@ -115,7 +115,7 @@ module V1 end it "does not authenticate when the app does not match the cookie" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} # cookie was set for the portal (no app), so the observations-tool app # cannot read it @@ -124,18 +124,8 @@ module V1 expect(status).to eq(401) end - it "Authorization header takes precedence over cookie" do - other_user = create(:admin) - post "/login", params: {auth: {email: other_user.email, password: "Supersecret1", set_cookie: true}} - - get "/users/current-user", headers: user_headers - - expect(status).to eq(200) - expect(parsed_attributes[:email]).to eq(user.email) - end - it "logout clears the auth cookie" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} expect(response.cookies[APIController::AUTH_COOKIE_NAME]).to be_present delete "/logout", headers: {APIController::CSRF_HEADER => cookies[APIController::CSRF_COOKIE_NAME]} @@ -147,7 +137,7 @@ module V1 describe "CSRF protection" do it "issues a non-HTTP-only XSRF-TOKEN cookie at login" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} expect(response.cookies[APIController::CSRF_COOKIE_NAME]).to be_present csrf_set_cookie = Array(response.headers["Set-Cookie"]).find { |c| c.start_with?("#{APIController::CSRF_COOKIE_NAME}=") } @@ -155,7 +145,7 @@ module V1 end it "issues a URL-safe token so the frontend can echo it back verbatim" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} csrf_set_cookie = Array(response.headers["Set-Cookie"]).find { |c| c.start_with?("#{APIController::CSRF_COOKIE_NAME}=") } value = csrf_set_cookie.split("=", 2).last.split(";").first @@ -164,7 +154,7 @@ module V1 end it "blocks cookie-authenticated unsafe requests without the CSRF header" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} delete "/logout" @@ -172,7 +162,7 @@ module V1 end it "blocks cookie-authenticated unsafe requests with a mismatched CSRF header" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} delete "/logout", headers: {APIController::CSRF_HEADER => "not-the-real-token"} @@ -180,7 +170,7 @@ module V1 end it "allows cookie-authenticated unsafe requests when the header matches" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} delete "/logout", headers: {APIController::CSRF_HEADER => cookies[APIController::CSRF_COOKIE_NAME]} @@ -188,7 +178,7 @@ module V1 end it "rejects a matching cookie and header that is not signed by the app" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} forged = "not-a-signed-token" cookies[APIController::CSRF_COOKIE_NAME] = forged @@ -199,7 +189,7 @@ module V1 it "rejects a token signed for another user" do other_user = create(:admin) - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} other_token = CsrfProtection.verifier .generate({"user_id" => other_user.id, "nonce" => SecureRandom.urlsafe_base64(16)}) @@ -210,21 +200,15 @@ module V1 end it "exempts safe (GET) cookie-authenticated requests from CSRF" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} get "/users/current-user" expect(status).to eq(200) end - it "exempts Bearer-authenticated requests from CSRF" do - delete "/logout", headers: user_headers - - expect(status).to eq(204) - end - it "re-issues the XSRF-TOKEN cookie when it's missing on a cookie-authed request" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} # simulate the XSRF cookie being cleared while the auth cookie persists cookies.delete(APIController::CSRF_COOKIE_NAME) @@ -238,7 +222,7 @@ module V1 end it "logout clears the XSRF-TOKEN cookie" do - post "/login", params: {auth: {email: user.email, password: "Supersecret1", set_cookie: true}} + post "/login", params: {auth: {email: user.email, password: "Supersecret1"}} delete "/logout", headers: {APIController::CSRF_HEADER => cookies[APIController::CSRF_COOKIE_NAME]} @@ -256,7 +240,7 @@ module V1 delete "/logout", headers: user_headers expect(status).to eq(204) - expect(response.headers["Set-Cookie"]).to include("download_user=;") + expect(response.headers["Set-Cookie"]).to include(a_string_starting_with("download_user=;")) expect(response.cookies["download_user"]).to be_blank end @@ -271,7 +255,8 @@ module V1 end it "Download session set download cookie for different app" do - post "/sessions/download-session?app=observations-tool", headers: user_headers + post "/sessions/download-session?app=observations-tool", + headers: authorize_headers(user.id, app: "observations-tool") expect(status).to eq(200) expect(response.cookies["observations-tool_download_user"]).to be_present diff --git a/spec/support/integration_helper.rb b/spec/support/integration_helper.rb index bb40b029e..d2aef4cf4 100644 --- a/spec/support/integration_helper.rb +++ b/spec/support/integration_helper.rb @@ -1,4 +1,6 @@ module IntegrationHelper + FACTORY_PASSWORD = "Supersecret1" + ERRORS = { "401" => {status: 401, title: "You are not authorized to access this page."}, "422" => {status: 422, title: "Unprocessable entity."}, @@ -39,16 +41,12 @@ def login_user(user) headers: jsonapi_headers) end - def generate_token(id) - JWT.encode({user: id}, ENV["AUTH_SECRET"], "HS256") - end - def admin @admin ||= create(:admin) end def admin_headers - @admin_headers ||= authorize_headers(admin.id) + authorize_headers(admin.id) end def user @@ -56,7 +54,7 @@ def user end def user_headers - @user_headers ||= authorize_headers(user.id) + authorize_headers(user.id) end def operator_user @@ -64,13 +62,27 @@ def operator_user end def operator_user_headers - @operator_user_headers ||= authorize_headers(operator_user.id) - end + authorize_headers(operator_user.id) + end + + # Logs the user in for real and hands back the headers a browser would send. + # The auth cookie lands in the shared jar as a side effect, so the login has + # to happen immediately before the request it authorizes — hence no + # memoization on the *_headers helpers: whichever role logged in last owns + # the jar, and re-logging in per call keeps each request honest. + # + # Every user factory inherits the same password, so an id is enough to log in. + def authorize_headers(id, jsonapi: true, app: nil) + user = User.find(id) + url = "/login" + url += "?app=#{app}" if app.present? + post url, params: {auth: {email: user.email, password: FACTORY_PASSWORD}} - def authorize_headers(id, jsonapi: true) - headers = { - "Authorization" => "Bearer #{generate_token(id)}" - } + # both cookies are namespaced per app, so a request sent with ?app= only + # authenticates against a login made for that same app + csrf_cookie_name = [app, APIController::CSRF_COOKIE_NAME].compact.join("_") + # unsafe requests need the double-submit token echoed back from the cookie + headers = {APIController::CSRF_HEADER => cookies[csrf_cookie_name]} headers.merge!(jsonapi_headers) if jsonapi headers end