Remove token auth

.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=
 

.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
 

Gemfile

@@ -72,7 +72,6 @@ gem "globalize"
 # Auth and Omniauth
 gem "bcrypt"
 gem "cancancan"
-gem "jwt"
 
 # Uploads
 gem "carrierwave-base64"

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

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]

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?

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)

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

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) }

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) }

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: [

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,15 +232,15 @@ 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
 
           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)