From 4b14e1a75c74f3348619876b8648eebe57e183f8 Mon Sep 17 00:00:00 2001 From: Alistair Davidson Date: Tue, 2 Sep 2025 15:27:28 +0100 Subject: [PATCH] Refactoring - move JWT-generation methods out of the controller and into the model, and add a constant to define the algorithm used for signing the JWTs in just one place --- .../reporting/one_time_tokens_controller.rb | 24 +------- .../token_authentication_concern.rb | 2 +- app/models/reporting_api/one_time_token.rb | 20 +++++++ .../one_time_tokens_controller_spec.rb | 10 +++- .../api/reporting/totals_controller_spec.rb | 6 +- .../token_authentication_concern_spec.rb | 4 +- .../reporting_api/one_time_token_spec.rb | 55 ++++++++++++++++++- 7 files changed, 89 insertions(+), 32 deletions(-) diff --git a/app/controllers/api/reporting/one_time_tokens_controller.rb b/app/controllers/api/reporting/one_time_tokens_controller.rb index ef5d199f05..34ddb752fd 100644 --- a/app/controllers/api/reporting/one_time_tokens_controller.rb +++ b/app/controllers/api/reporting/one_time_tokens_controller.rb @@ -9,7 +9,7 @@ class API::Reporting::OneTimeTokensController < API::Reporting::BaseController def authorize @token = ReportingAPI::OneTimeToken.find_by!(token: params[:code]) @token.delete # <- Tokens are one-time use - json_data = { jwt: jwt(@token) } + json_data = { jwt: @token.to_jwt } render json: json_data rescue ActiveRecord::RecordNotFound render json: { errors: "invalid_grant" }, status: :forbidden @@ -23,26 +23,4 @@ def verify_grant_type! return end end - - def jwt_payload(token) - { - "iat" => Time.current.utc.to_i, - "data" => { - "user" => token.user.as_json, - "cis2_info" => token.cis2_info - } - } - end - - def jwt(token) - JWT.encode( - jwt_payload(token), - Settings.reporting_api.client_app.secret, - "HS512" - ) - end - - def ensure_reporting_api_feature_enabled - render status: :forbidden and return unless Flipper.enabled?(:reporting_api) - end end diff --git a/app/controllers/concerns/reporting_api/token_authentication_concern.rb b/app/controllers/concerns/reporting_api/token_authentication_concern.rb index 354abef964..cb13d5dab0 100644 --- a/app/controllers/concerns/reporting_api/token_authentication_concern.rb +++ b/app/controllers/concerns/reporting_api/token_authentication_concern.rb @@ -64,7 +64,7 @@ def decode_jwt!(jwt) jwt, Settings.reporting_api.client_app.secret, true, - { algorithm: "HS512" } + { algorithm: ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM } ) end end diff --git a/app/models/reporting_api/one_time_token.rb b/app/models/reporting_api/one_time_token.rb index 89203d83e7..39e4fbd658 100644 --- a/app/models/reporting_api/one_time_token.rb +++ b/app/models/reporting_api/one_time_token.rb @@ -26,6 +26,8 @@ class ReportingAPI::OneTimeToken < ApplicationRecord validates :user_id, uniqueness: true, presence: true validates :token, uniqueness: true, presence: true + JWT_SIGNING_ALGORITHM = "HS512" + def self.generate!(user_id:, cis2_info: {}) create!(user_id: user_id, token: SecureRandom.hex(32), cis2_info: cis2_info) end @@ -46,4 +48,22 @@ def self.find_or_generate_for!(user:, cis2_info: {}) def expired? created_at < self.class.expire_before end + + def jwt_payload + { + "iat" => Time.current.utc.to_i, + "data" => { + "user" => user.as_json, + "cis2_info" => cis2_info + } + } + end + + def to_jwt + JWT.encode( + jwt_payload, + Settings.reporting_api.client_app.secret, + JWT_SIGNING_ALGORITHM + ) + end end diff --git a/spec/controllers/api/reporting/one_time_tokens_controller_spec.rb b/spec/controllers/api/reporting/one_time_tokens_controller_spec.rb index 60110c1f67..bfa9c71d2d 100644 --- a/spec/controllers/api/reporting/one_time_tokens_controller_spec.rb +++ b/spec/controllers/api/reporting/one_time_tokens_controller_spec.rb @@ -95,7 +95,10 @@ response_json["jwt"], Settings.reporting_api.client_app.secret, true, - { algorithm: "HS512" } + { + algorithm: + ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM + } ) }.not_to raise_error end @@ -106,7 +109,10 @@ response_json["jwt"], Settings.reporting_api.client_app.secret, true, - { algorithm: "HS512" } + { + algorithm: + ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM + } ) end let(:jwt_data) { decoded_payload.first["data"] } diff --git a/spec/controllers/api/reporting/totals_controller_spec.rb b/spec/controllers/api/reporting/totals_controller_spec.rb index b654f92219..5fc327d195 100644 --- a/spec/controllers/api/reporting/totals_controller_spec.rb +++ b/spec/controllers/api/reporting/totals_controller_spec.rb @@ -33,7 +33,7 @@ JWT.encode( valid_payload, Settings.reporting_api.client_app.secret, - "HS512" + ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM ) end @@ -58,7 +58,7 @@ JWT.encode( valid_payload, Settings.reporting_api.client_app.secret, - "HS512" + ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM ) end @@ -73,7 +73,7 @@ JWT.encode( invalid_payload, Settings.reporting_api.client_app.secret, - "HS512" + ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM ) end diff --git a/spec/controllers/concerns/reporting_api/token_authentication_concern_spec.rb b/spec/controllers/concerns/reporting_api/token_authentication_concern_spec.rb index 96a82f9b95..67372af46d 100644 --- a/spec/controllers/concerns/reporting_api/token_authentication_concern_spec.rb +++ b/spec/controllers/concerns/reporting_api/token_authentication_concern_spec.rb @@ -273,7 +273,7 @@ def current_user jwt, Settings.reporting_api.client_app.secret, true, - { algorithm: "HS512" } + { algorithm: ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM } ) #.and_return(decoded_jwt) an_object_which_includes_the_concern.send(:decode_jwt!, jwt) end @@ -284,7 +284,7 @@ def current_user jwt, Settings.reporting_api.client_app.secret, true, - { algorithm: "HS512" } + { algorithm: ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM } ).and_return(decoded_jwt) end diff --git a/spec/models/reporting_api/one_time_token_spec.rb b/spec/models/reporting_api/one_time_token_spec.rb index 48d38c8d95..0a376e7463 100644 --- a/spec/models/reporting_api/one_time_token_spec.rb +++ b/spec/models/reporting_api/one_time_token_spec.rb @@ -22,7 +22,9 @@ # describe ReportingAPI::OneTimeToken do - subject { described_class.new(user_id: user.id, token: SecureRandom.hex(32)) } + subject(:token) do + described_class.new(user_id: user.id, token: SecureRandom.hex(32)) + end let(:user) { create(:user) } @@ -168,4 +170,55 @@ end end end + + describe "#jwt_payload" do + let(:result) { token.jwt_payload } + + it "has iat set to the current time as an integer" do + expect(result["iat"]).to be_within(1000).of(Time.current.utc.to_i) + end + + it "has a data hash" do + expect(result["data"]).to be_a(Hash) + end + + describe "the data hash" do + let(:data) { result["data"] } + + it "has the user as json" do + expect(data["user"]).to eq(token.user.as_json) + end + + it "has the cis2_info as json" do + expect(data["cis2_info"]).to eq(token.cis2_info.as_json) + end + end + end + + describe "#to_jwt" do + let(:result) { token.to_jwt } + let(:decoded_payload) do + JWT.decode( + result, + Settings.reporting_api.client_app.secret, + true, + { algorithm: ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM } + ).first + end + + it "returns a JWT signed with the Mavis reporting app secret" do + expect { + JWT.decode( + result, + Settings.reporting_api.client_app.secret, + true, + { algorithm: ReportingAPI::OneTimeToken::JWT_SIGNING_ALGORITHM } + ) + }.not_to raise_error + end + + it "has the jwt_payload" do + expect(decoded_payload).to eq(token.jwt_payload) + end + end end