Merge pull request #2307 from MushroomObserver/nimmo-jwt-suggestions

Suggestions for JWT

app/controllers/observations/inat_imports_controller.rb

@@ -38,11 +38,11 @@ def create
       return designation_required unless imports_designated?
       return consent_required if params[:consent] == "0"
 
-      inat_import = InatImport.find_or_create_by(user: User.current)
-      inat_import.update(state: "Authorizing",
-                         import_all: params[:all],
-                         inat_ids: params[:inat_ids],
-                         inat_username: params[:inat_username])
+      @inat_import = InatImport.find_or_create_by(user: User.current)
+      @inat_import.update(state: "Authorizing",
+                          import_all: params[:all],
+                          inat_ids: params[:inat_ids],
+                          inat_username: params[:inat_username])
 
       request_inat_user_authorization
     end
@@ -107,23 +107,35 @@ def authenticate
       auth_code = params[:code]
       return not_authorized if auth_code.blank?
 
-      inat_import = InatImport.find_or_create_by(user: User.current)
-      inat_import.update(state: "Authenticating")
+      @inat_import = InatImport.find_or_create_by(user: User.current)
+      @inat_import.update(state: "Authenticating")
 
-      # exchange code received from iNat for an access token
+      # exchange code received from iNat for an oAuth `access_token`
       payload = {
         client_id: APP_ID,
         client_secret: Rails.application.credentials.inat.secret,
         code: auth_code,
         redirect_uri: REDIRECT_URI,
         grant_type: "authorization_code"
       }
-      response = RestClient.post("#{SITE}/oauth/token", payload)
-      inat_import.update(token: response.body, state: "Importing")
+      oauth_response = RestClient.post("#{SITE}/oauth/token", payload)
+      # The actual token is in the field ["access_token"].
+      access_token = JSON.parse(oauth_response.body)["access_token"]
+
+      # Use the `access_token` to request a `jwt`, right away.
+      jwt_response = RestClient::Request.execute(
+        method: :get, url: "https://www.inaturalist.org/users/api_token",
+        headers: { authorization: "Bearer #{access_token}", accept: :json }
+      )
+      api_token = JSON.parse(jwt_response)["api_token"]
+
+      # Now that we've got the right token, we can make authenticated requests
+      # to iNat that get the real real private data.
+      @inat_import.update(token: api_token, state: "Importing")
 
-      import_requested_observations(inat_import)
+      import_requested_observations
 
-      inat_import.update(state: "Done")
+      @inat_import.update(state: "Done")
       redirect_to(observations_path)
     end
 
@@ -136,16 +148,17 @@ def not_authorized
       redirect_to(observations_path)
     end
 
-    def import_requested_observations(inat_import)
-      inat_ids = inat_id_list(inat_import)
-      return if inat_import[:import_all].blank? && inat_ids.blank?
+    def import_requested_observations
+      inat_ids = inat_id_list
+      return if @inat_import[:import_all].blank? && inat_ids.blank?
 
       last_import_id = 0
       loop do
         page =
+          # make an iNat API search observations request
           inat_search_observations(
-            ids: inat_ids, id_above: last_import_id,
-            user_login: inat_import.inat_username
+            id: inat_ids, id_above: last_import_id,
+            user_login: @inat_import.inat_username
           )
         break if page_empty?(page)
 
@@ -167,22 +180,37 @@ def done_with_page?(parsed_page)
         parsed_page["page"] * parsed_page["per_page"]
     end
 
-    def inat_id_list(inat_import)
-      inat_import.inat_ids.delete(" ")
+    def inat_id_list
+      @inat_import.inat_ids.delete(" ")
     end
 
     # https://api.inaturalist.org/v1/docs/#!/Observations/get_observations
-    def inat_search_observations(ids: nil, id_above: nil, only_id: false,
-                                 per_page: 200, sort: "order=asc&order_by=id",
-                                 # prevents user from importing others' obss
-                                 user_login: nil)
-      operation =
-        "/observations?id=#{ids}&id_above=#{id_above}&only_id=#{only_id}" \
-        "&per_page=#{per_page}&#{sort}&user_login=#{user_login}" \
-        "&iconic_taxa=Fungi,Protozoa"
-      ::Inat.new(operation: operation, token: inat_import.token).body
+    # https://stackoverflow.com/a/11251654/3357635
+    # Note that the `ids` parameter may be a comma-separated list of iNat obs
+    # ids - that needs to be URL encoded to a string when passed as an arg here
+    # because URI.encode_www_form deals with arrays by passing the same key
+    # multiple times.
+    def inat_search_observations(**args)
+      query_args = {
+        id: nil, id_above: nil, only_id: false, per_page: 200,
+        order: "asc", order_by: "id",
+        # prevents user from importing others' obss
+        user_login: nil, iconic_taxa: ICONIC_TAXA
+      }.merge(args)
+
+      query = URI.encode_www_form(query_args)
+      # ::Inat.new(operation: query, token: @inat_import.token).body
+
+      # Nimmo 2024-06-19 jdc. Moving the request from the inat class to here.
+      # RestClient::Request.execute wasn't available in the class
+      headers = { authorization: "Bearer #{@inat_import.token}", accept: :json }
+      @inat = RestClient::Request.execute(
+        method: :get, url: "#{API_BASE}/observations?#{query}", headers: headers
+      )
     end
 
+    API_BASE = "https://api.inaturalist.org/v1"
+
     def import_page(page)
       JSON.parse(page)["results"].each do |result|
         import_one_result(JSON.generate(result))
@@ -222,9 +250,9 @@ def import_one_result(result)
       # update_field_slip(@observation, params[:field_code])
     end
 
-    def inat_import
-      InatImport.find_by(user: User.current)
-    end
+    # def find_inat_import
+    #   InatImport.find_by(user: User.current)
+    # end
 
     def not_importable(inat_obs)
       return if inat_obs.taxon_importable?

db/schema.rb

@@ -85,7 +85,7 @@
     t.integer "project_id"
   end
 
-  create_table "field_slip_job_trackers", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "field_slip_job_trackers", charset: "utf8mb3", force: :cascade do |t|
     t.integer "start"
     t.integer "count"
     t.string "prefix"
@@ -637,7 +637,7 @@
     t.datetime "updated_at", precision: nil, null: false
   end
 
-  create_table "solid_queue_blocked_executions", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_blocked_executions", charset: "utf8mb3", force: :cascade do |t|
     t.bigint "job_id", null: false
     t.string "queue_name", null: false
     t.integer "priority", default: 0, null: false
@@ -649,22 +649,22 @@
     t.index ["job_id"], name: "index_solid_queue_blocked_executions_on_job_id", unique: true
   end
 
-  create_table "solid_queue_claimed_executions", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_claimed_executions", charset: "utf8mb3", force: :cascade do |t|
     t.bigint "job_id", null: false
     t.bigint "process_id"
     t.datetime "created_at", null: false
     t.index ["job_id"], name: "index_solid_queue_claimed_executions_on_job_id", unique: true
     t.index ["process_id", "job_id"], name: "index_solid_queue_claimed_executions_on_process_id_and_job_id"
   end
 
-  create_table "solid_queue_failed_executions", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_failed_executions", charset: "utf8mb3", force: :cascade do |t|
     t.bigint "job_id", null: false
     t.text "error"
     t.datetime "created_at", null: false
     t.index ["job_id"], name: "index_solid_queue_failed_executions_on_job_id", unique: true
   end
 
-  create_table "solid_queue_jobs", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_jobs", charset: "utf8mb3", force: :cascade do |t|
     t.string "queue_name", null: false
     t.string "class_name", null: false
     t.text "arguments"
@@ -682,13 +682,13 @@
     t.index ["scheduled_at", "finished_at"], name: "index_solid_queue_jobs_for_alerting"
   end
 
-  create_table "solid_queue_pauses", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_pauses", charset: "utf8mb3", force: :cascade do |t|
     t.string "queue_name", null: false
     t.datetime "created_at", null: false
     t.index ["queue_name"], name: "index_solid_queue_pauses_on_queue_name", unique: true
   end
 
-  create_table "solid_queue_processes", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_processes", charset: "utf8mb3", force: :cascade do |t|
     t.string "kind", null: false
     t.datetime "last_heartbeat_at", null: false
     t.bigint "supervisor_id"
@@ -700,7 +700,7 @@
     t.index ["supervisor_id"], name: "index_solid_queue_processes_on_supervisor_id"
   end
 
-  create_table "solid_queue_ready_executions", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_ready_executions", charset: "utf8mb3", force: :cascade do |t|
     t.bigint "job_id", null: false
     t.string "queue_name", null: false
     t.integer "priority", default: 0, null: false
@@ -710,7 +710,7 @@
     t.index ["queue_name", "priority", "job_id"], name: "index_solid_queue_poll_by_queue"
   end
 
-  create_table "solid_queue_recurring_executions", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_recurring_executions", charset: "utf8mb3", force: :cascade do |t|
     t.bigint "job_id", null: false
     t.string "task_key", null: false
     t.datetime "run_at", null: false
@@ -719,7 +719,7 @@
     t.index ["task_key", "run_at"], name: "index_solid_queue_recurring_executions_on_task_key_and_run_at", unique: true
   end
 
-  create_table "solid_queue_scheduled_executions", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_scheduled_executions", charset: "utf8mb3", force: :cascade do |t|
     t.bigint "job_id", null: false
     t.string "queue_name", null: false
     t.integer "priority", default: 0, null: false
@@ -729,7 +729,7 @@
     t.index ["scheduled_at", "priority", "job_id"], name: "index_solid_queue_dispatch_all"
   end
 
-  create_table "solid_queue_semaphores", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+  create_table "solid_queue_semaphores", charset: "utf8mb3", force: :cascade do |t|
     t.string "key", null: false
     t.integer "value", default: 1, null: false
     t.datetime "expires_at", null: false

test/controllers/observations/inat_imports_controller_test.rb

@@ -110,7 +110,8 @@ def test_import_authorized
       inat_import.save
       inat_authorization_callback_params = { code: "MockCode" }
 
-      stub_token_request
+      stub_access_token_request
+      stub_jwt_request
       login(user.login)
       get(:authenticate, params: inat_authorization_callback_params)
 
@@ -299,7 +300,8 @@ def test_import_plant
 
       stub_inat_api_request(inat_import_ids, mock_search_result)
       simulate_authorization(user: user, inat_import_ids: inat_import_ids)
-      stub_token_request
+      stub_access_token_request
+      stub_jwt_request
 
       params = { inat_ids: inat_import_ids, code: "MockCode" }
       login(user.login)
@@ -321,13 +323,15 @@ def test_import_zero_results
 
       stub_inat_api_request(inat_import_ids, mock_search_result)
       simulate_authorization(user: user, inat_import_ids: inat_import_ids)
-      stub_token_request
+      stub_access_token_request
+      stub_jwt_request
 
       params = { inat_ids: inat_import_ids, code: "MockCode" }
       login(user.login)
 
       assert_no_difference(
-        "Observation.count", "Should not import iNat Plant observations"
+        "Observation.count",
+        "Should not import if there's no iNat obs with a matching id"
       ) do
         post(:authenticate, params: params)
       end
@@ -351,7 +355,7 @@ def test_import_multiple
       assert_equal(231_104_466, json["results"].second["id"])
 
       simulate_authorization(user: user, inat_import_ids: inat_import_ids)
-      stub_token_request
+      stub_access_token_request
       stub_inat_api_request(inat_import_ids, mock_inat_response)
 
       params = { inat_ids: inat_import_ids, code: "MockCode" }
@@ -379,7 +383,7 @@ def test_import_all
 
       simulate_authorization(user: user, inat_import_ids: inat_import_ids,
                              import_all: true)
-      stub_token_request
+      stub_access_token_request
       stub_inat_api_request(inat_import_ids, mock_search_result)
 
       params = { inat_ids: inat_import_ids, code: "MockCode" }
@@ -416,9 +420,10 @@ def import_mock_observation(filename)
       simulate_authorization(user: user,
                              inat_username: inat_obs.inat_user_login,
                              inat_import_ids: inat_import_ids)
-      stub_token_request
+      stub_access_token_request
       stub_inat_api_request(inat_import_ids, mock_search_result,
                             inat_user_login: inat_obs.inat_user_login)
+      stub_jwt_request
 
       params = { inat_ids: inat_import_ids, code: "MockCode" }
       login(user.login)
@@ -437,23 +442,24 @@ def import_mock_observation(filename)
 
     def stub_inat_api_request(inat_obs_ids, mock_inat_response, id_above: 0,
                               inat_user_login: nil)
-      # mirror param order of iNat example/test page
-      # https://api.inaturalist.org/v1/docs/#!/Observations/get_observations
-      # for convenience in creating mock responses from that page
-      params = <<~PARAMS.delete("\n")
-        ?id=#{inat_obs_ids}
-        &user_login=#{inat_user_login}
-        &iconic_taxa=#{Observations::InatImportsController::ICONIC_TAXA}
+      # params must be in same order as in the controller
+      # omit trailing "=" since the controller omits it (via `merge`)
+      params = <<~PARAMS.delete("\n").chomp("=")
+        ?iconic_taxa=#{Observations::InatImportsController::ICONIC_TAXA}
+        &id=#{inat_obs_ids}
         &id_above=#{id_above}
         &per_page=200
-        &order=asc&order_by=id
         &only_id=false
+        &order=asc&order_by=id
+        &user_login=#{inat_user_login}
       PARAMS
-
-      WebMock.stub_request(
-        :get,
-        "#{INAT_OBS_REQUEST_PREFIX}#{params}"
-      ).to_return(body: mock_inat_response)
+      stub_request(:get, "#{INAT_OBS_REQUEST_PREFIX}#{params}").
+        with(headers:
+        { "Accept" => "application/json",
+          "Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3",
+          "Authorization" => "Bearer",
+          "Host" => "api.inaturalist.org" }).
+        to_return(body: mock_inat_response)
     end
 
     def mock_photo_importer(inat_obs)
@@ -515,7 +521,7 @@ def authorization_url
 
     # stub exchanging iNat code for oauth token
     # https://www.inaturalist.org/pages/api+reference#authorization_code_flow
-    def stub_token_request
+    def stub_access_token_request
       stub_request(:post, "https://www.inaturalist.org/oauth/token").
         with(
           body: { "client_id" => Rails.application.credentials.inat.id,
@@ -524,7 +530,24 @@ def stub_token_request
                   "grant_type" => "authorization_code",
                   "redirect_uri" => REDIRECT_URI }
         ).
-        to_return(status: 200, body: "MockToken", headers: {})
+        to_return(status: 200,
+                  body: { access_token: "MockAccessToken" }.to_json,
+                  headers: {})
+    end
+
+    def stub_jwt_request
+      stub_request(:get, "https://www.inaturalist.org/users/api_token").
+        with(
+          headers: {
+            "Accept" => "application/json",
+            "Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3",
+            "Authorization" => "Bearer MockAccessToken",
+            "Host" => "www.inaturalist.org"
+          }
+        ).
+        to_return(status: 200,
+                  body: { access_token: "MockJWT" }.to_json,
+                  headers: {})
     end
 
     def result_without_photos(mock_search_result)