Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kev ma/current usr mgmt cc rsec fixes #1718

Open
wants to merge 37 commits into
base: current-user-mgmt
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
5b3c218
Add Pre File Fetch Sensitivity Check (#1652)
tradesmanhelix Aug 2, 2024
d9a2f77
Add Specs to Cover Additional Sensitivity Cases (#1662)
tradesmanhelix Aug 13, 2024
9557efd
Gate VBMS Methods with Sensitivity Checks (#1665)
tradesmanhelix Aug 13, 2024
f1d396f
Remove Unneeded Sensitivity Checks (#1666)
tradesmanhelix Aug 14, 2024
af9d251
Restore Pre-Fetch Sensitivity Check Logic & Add Frontend Feature Togg…
tradesmanhelix Aug 15, 2024
e9fd8ab
Updated ruby_claim_evidence_api gem
youfoundmanesh Aug 15, 2024
01576e4
Fix Missing Manifest User Causing Sensitivity Check Failures (#1675)
tradesmanhelix Sep 4, 2024
2f41563
Use New Sensitivity Check Method (#1676)
tradesmanhelix Sep 6, 2024
183286b
Update Specs to Address Failures (#1677)
tradesmanhelix Sep 6, 2024
c3d84ce
Send Veteran Number in Restart Request (#1679)
tradesmanhelix Sep 6, 2024
1a22df9
Update CE API Gem (#1680)
tradesmanhelix Sep 6, 2024
0555acf
Update Logic for Setting Veteran ID in Request (#1681)
tradesmanhelix Sep 9, 2024
cb43002
Update JSON Parsing Logic to Handle CE API Gem Changes (#1686)
tradesmanhelix Sep 11, 2024
4a4f847
Updated ruby_claim_evidence_api
youfoundmanesh Sep 12, 2024
9e2a9a4
Add current_user to SaveFilesInS3 Job (#1688)
tradesmanhelix Sep 13, 2024
6d18455
add check for empty result (#1687)
SanthiParakal133 Sep 13, 2024
1eaf33c
Add Script for Finding Valid UAT Testing Users (#1690)
tradesmanhelix Sep 13, 2024
9322a50
Kev ma/appeals 58216 time adjust (#1692)
Kevma50287 Sep 16, 2024
89a91f0
Fix Incorrect Keyword in Script (#1693)
tradesmanhelix Sep 16, 2024
455fef7
Handle All Possible Errors in Script (#1694)
tradesmanhelix Sep 16, 2024
ae2f385
Improve Script Output (#1695)
tradesmanhelix Sep 16, 2024
8b83f87
Updated ruby_claim_evidence_api gem with ref
youfoundmanesh Sep 23, 2024
c13cb12
Update feature toogle to send_current_user_cred
youfoundmanesh Sep 25, 2024
b701318
Update feature toogle to send_current_user_cred_to_ce_api
youfoundmanesh Sep 26, 2024
919352c
Kev ma/appeals 59461 (#1701)
Kevma50287 Sep 27, 2024
50d2d0c
typo fix (#1702)
Kevma50287 Sep 27, 2024
05044a2
Kev ma/appeals 58827 v2 (#1703)
Kevma50287 Sep 30, 2024
6b58ecc
Updated ruby_claim_evidence_api gem
youfoundmanesh Aug 15, 2024
c31b56d
Updated ruby_claim_evidence_api
youfoundmanesh Sep 12, 2024
7f74f46
Deepak/appeals 59642 v1 (#1714)
SanthiParakal133 Oct 9, 2024
3461418
Fix rspec - CE API verifies w/ vet file number, old w/ vet id
Kevma50287 Oct 9, 2024
3aae93a
added missing feature toggles to revert to prev funcitonality
Kevma50287 Oct 9, 2024
8f45925
CC Refactor
Kevma50287 Oct 9, 2024
39c912d
Merge branch 'main' into current-user-mgmt
youfoundmanesh Oct 10, 2024
011ca6e
CC - refactor logic into method for readability
Kevma50287 Oct 10, 2024
16b306c
Linting update
Kevma50287 Oct 10, 2024
8691a77
Merge branch 'current-user-mgmt' into KevMa/current-usr-mgmt-cc-rsec-…
Kevma50287 Oct 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ gem "redis-namespace"
gem "redis-rails", "~> 5.0.2"
gem "redis-semaphore"
gem "request_store"
gem "ruby_claim_evidence_api", git: "https://github.com/department-of-veterans-affairs/ruby_claim_evidence_api.git", ref: "095798918338650383b06ff535bc63fc5fbfc8dc"
gem "rubyzip", ">= 1.3.0"
gem "ruby_claim_evidence_api", git: "https://github.com/department-of-veterans-affairs/ruby_claim_evidence_api.git", branch: "current-user-mgmt"
gem "sass-rails", "~> 5.0"
gem "sentry-raven"
gem "shoryuken", "3.1.11"
Expand Down
20 changes: 8 additions & 12 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ GIT

GIT
remote: https://github.com/department-of-veterans-affairs/ruby_claim_evidence_api.git
revision: 095798918338650383b06ff535bc63fc5fbfc8dc
ref: 095798918338650383b06ff535bc63fc5fbfc8dc
revision: 9e667a1e626cbaf29f3f759d93e93e1afb13ab16
branch: current-user-mgmt
specs:
ruby_claim_evidence_api (0.1.0)
activesupport
Expand Down Expand Up @@ -225,7 +225,7 @@ GEM
coffee-script-source
execjs
coffee-script-source (1.12.2)
concurrent-ruby (1.3.3)
concurrent-ruby (1.3.4)
crack (1.0.0)
bigdecimal
rexml
Expand Down Expand Up @@ -261,7 +261,7 @@ GEM
google-protobuf (>= 3.18, < 5.a)
gyoku (1.3.1)
builder (>= 2.1.2)
hashdiff (1.1.0)
hashdiff (1.1.1)
hashie (4.1.0)
httparty (0.18.1)
mime-types (~> 3.0)
Expand Down Expand Up @@ -308,8 +308,7 @@ GEM
mime-types-data (3.2020.1104)
mini_magick (4.11.0)
mini_mime (1.1.5)
mini_portile2 (2.8.7)
minitest (5.24.1)
minitest (5.25.1)
moment_timezone-rails (0.5.14)
momentjs-rails (~> 2.15.1)
momentjs-rails (2.15.1)
Expand All @@ -330,7 +329,6 @@ GEM
net-protocol
nio4r (2.7.3)
nokogiri (1.15.6)
mini_portile2 (~> 2.8.2)
racc (~> 1.4)
nori (2.6.0)
omniauth (1.9.1)
Expand Down Expand Up @@ -440,7 +438,7 @@ GEM
public_suffix (5.1.1)
puma (5.6.4)
nio4r (~> 2.0)
racc (1.8.0)
racc (1.8.1)
rack (2.2.9)
rack-cors (1.1.1)
rack (>= 2.0.0)
Expand Down Expand Up @@ -507,8 +505,7 @@ GEM
regexp_parser (2.8.3)
request_store (1.5.0)
rack (>= 1.4)
rexml (3.3.1)
strscan
rexml (3.3.8)
rspec (3.10.0)
rspec-core (~> 3.10.0)
rspec-expectations (~> 3.10.0)
Expand Down Expand Up @@ -600,7 +597,6 @@ GEM
activesupport (>= 6.1)
sprockets (>= 3.0.0)
statsd-instrument (3.7.0)
strscan (3.1.0)
systemu (2.6.5)
thor (1.3.1)
tilt (2.0.11)
Expand Down Expand Up @@ -645,7 +641,7 @@ GEM
xpath (3.2.0)
nokogiri (~> 1.8)
zaru (0.3.0)
zeitwerk (2.6.16)
zeitwerk (2.6.18)
zero_downtime_migrations (0.0.7)
activerecord

Expand Down
10 changes: 10 additions & 0 deletions app/assets/stylesheets/_commons.scss
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,16 @@ dd {
}
}

.cf-icon-external-link {
vertical-align: -.45ex;
width: 1em;
height: 1em;

path {
fill: $cf-link;
}
}

.cf-icon-close {
display: block;
margin: auto;
Expand Down
11 changes: 10 additions & 1 deletion app/controllers/api/v1/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ class Api::V1::ApplicationController < BaseController
}, status: 500
end

rescue_from BGS::SensitivityLevelCheckFailure do |e|
render json: {
status: e.message,
featureToggles: {
use_ce_api: FeatureToggle.enabled?(:use_ce_api)
}
}, status: :forbidden
end

rescue_from BGS::PublicError do |error|
forbidden(error.public_message)
end
Expand All @@ -28,7 +37,7 @@ def sensitive_record
end

def forbidden(reason = "Forbidden: unspecified")
render json: { status: reason }, status: 403
render json: { status: reason }, status: :forbidden
end

def missing_header(header)
Expand Down
21 changes: 20 additions & 1 deletion app/controllers/api/v2/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ def verify_veteran_file_number

return invalid_file_number unless bgs_service.valid_file_number?(file_number)

fetch_veteran_by_file_number(file_number)
if FeatureToggle.enabled?(:use_ce_api)
ce_api_fetch_veteran_file_number(file_number)
else
fetch_veteran_by_file_number(file_number)
end
end

def fetch_veteran_by_file_number(file_number)
Expand All @@ -41,4 +45,19 @@ def fetch_veteran_by_file_number(file_number)
raise BGS::ShareError, "Cannot access Veteran record"
end
end

def ce_api_fetch_veteran_file_number(file_number)
if sensitivity_checker.sensitivity_levels_compatible?(
user: current_user,
veteran_file_number: file_number
)
file_number
else
raise BGS::SensitivityLevelCheckFailure.new, "You are not authorized to access this file number"
end
end

def sensitivity_checker
@sensitivity_checker ||= SensitivityChecker.new
end
end
48 changes: 43 additions & 5 deletions app/controllers/api/v2/manifests_controller.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,33 @@
# frozen_string_literal: true

class Api::V2::ManifestsController < Api::V2::ApplicationController
# Need this as a before action since it gates access to these controller methods
before_action :veteran_file_number, only: [:start, :refresh]

def start
file_number = verify_veteran_file_number
return if performed?
if FeatureToggle.enabled?(:use_ce_api)
return if performed?

manifest = Manifest.includes(:sources, :records).find_or_create_by_user(user: current_user, file_number: veteran_file_number)
else
file_number = verify_veteran_file_number
return if performed?

manifest = Manifest.includes(:sources, :records).find_or_create_by_user(user: current_user, file_number: file_number)
end

manifest = Manifest.includes(:sources, :records).find_or_create_by_user(user: current_user, file_number: file_number)
manifest.start!
render json: json_manifests(manifest)
end

def refresh
manifest = Manifest.find(params[:id])
return record_not_found unless manifest
manifest = if FeatureToggle.enabled?(:use_ce_api)
Manifest.includes(:sources, :records).find_or_create_by_user(user: current_user, file_number: veteran_file_number)
else
Manifest.find(params[:id])
end

return record_not_found if manifest.blank?
return sensitive_record unless manifest.files_downloads.find_by(user: current_user)

manifest.start!
Expand All @@ -22,7 +39,9 @@ def progress
distribute_reads do
files_download ||= FilesDownload.find_with_manifest(manifest_id: params[:id], user_id: current_user.id)
end

return record_not_found unless files_download

render json: json_manifests(files_download.manifest)
end

Expand All @@ -32,6 +51,25 @@ def history

private

def veteran_file_number
return if !FeatureToggle.enabled?(:use_ce_api)

@veteran_file_number ||= verify_veteran_file_number
end

def verify_veteran_file_number
# The frontend may not have set this value (needed by parent's verify_veteran_file_number)
# but we are still able to determine it here in the child using the manifest
if request.headers["HTTP_FILE_NUMBER"].blank?
if params[:id].present?
manifest = Manifest.find(params[:id])
request.headers["HTTP_FILE_NUMBER"] = manifest.file_number
end
end

super
end

def json_manifests(manifest)
ActiveModelSerializers::SerializableResource.new(
manifest,
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/base_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
# frozen_string_literal: true

require "bgs"
require "bgs_errors"

class BaseController < ActionController::Base
before_action :strict_transport_security
before_action :current_user
Expand Down
1 change: 1 addition & 0 deletions app/exceptions/bgs_errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ class InvalidApplication < StandardError; end
class NoActiveStations < StandardError; end
class NoCaseflowAccess < StandardError; end
class StationAssertionRequired < StandardError; end
class SensitivityLevelCheckFailure < StandardError; end
end
4 changes: 2 additions & 2 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# frozen_string_literal: true

# rubocop:disable Metrics/ModuleLength
module ApplicationHelper
def ui_user?
return false unless RequestStore[:current_user]

(RequestStore[:current_user].roles || []).include?("Download eFolder")
end

Expand All @@ -13,6 +13,7 @@ def current_ga_path
begin
route = Rails.application.routes.recognize_path(full_path)
return full_path unless route

["", route[:controller], route[:action]].join("/")

# no match in recognize_path
Expand All @@ -21,4 +22,3 @@ def current_ga_path
end
end
end
# rubocop:enable Metrics/ModuleLength
10 changes: 5 additions & 5 deletions app/jobs/v2/download_manifest_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ class V2::DownloadManifestJob < ApplicationJob
def perform(manifest_source, user = nil)
manifest_source.update(status: :pending)

RequestStore.store[:current_user] = user if user
RequestStore.store[:current_user] = user if user.present?
Raven.extra_context(manifest_source: manifest_source.id)

documents = ManifestFetcher.new(manifest_source: manifest_source).process

log_info(manifest_source.manifest.file_number, documents, manifest_source.manifest.zipfile_size)

V2::SaveFilesInS3Job.perform_later(manifest_source) if documents.present? && !ApplicationController.helpers.ui_user?
rescue StandardError => error
V2::SaveFilesInS3Job.perform_later(manifest_source, current_user&.id) if documents.present? && !ApplicationController.helpers.ui_user?
rescue StandardError => e
manifest_source.update!(status: :failed)
Rails.logger.error "DownloadManifestJob encountered error #{error.class.name} when fetching manifest for appeal #{manifest_source.manifest.file_number}"
raise error
Rails.logger.error "DownloadManifestJob encountered error #{e.class.name} when fetching manifest for appeal #{manifest_source.manifest.file_number}"
raise e
end

def max_attempts
Expand Down
10 changes: 8 additions & 2 deletions app/jobs/v2/save_files_in_s3_job.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
class V2::SaveFilesInS3Job < ApplicationJob
queue_as :low_priority

def perform(manifest_source)
Raven.extra_context(manifest_source: manifest_source.id)
def perform(manifest_source, user_id = nil)
Raven.extra_context(manifest_source: manifest_source.id, user_id: user_id)

# Set user for permission check if the user is blank
if FeatureToggle.enabled?(:use_ce_api) && RequestStore[:current_user].blank?
user = User.find(user_id)
RequestStore.store[:current_user] = user
end

manifest_source.records.each(&:fetch!)
end
Expand Down
13 changes: 11 additions & 2 deletions app/models/manifest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ class Manifest < ApplicationRecord
failed: 3
}

UI_HOURS_UNTIL_EXPIRY = 72
API_HOURS_UNTIL_EXPIRY = 3
UI_HOURS_UNTIL_EXPIRY = Rails.non_production_env? && Rails.non_test_env? ? 0.25 : 72
API_HOURS_UNTIL_EXPIRY = Rails.non_production_env? && Rails.non_test_env? ? 0.25 : 3

SECONDS_TO_AUTO_UNLOCK = 5

Expand All @@ -30,6 +30,9 @@ class Manifest < ApplicationRecord
def start!
# Reset stale manifests.
update!(fetched_files_status: :initialized) if ready_for_refresh?
raise BGS::SensitivityLevelCheckFailure.new, "You are not authorized to access this manifest" if
FeatureToggle.enabled?(:use_ce_api) &&
!sensitivity_checker.sensitivity_levels_compatible?(user: user, veteran_file_number: file_number)

vbms_source.start!
vva_source.start! unless FeatureToggle.enabled?(:skip_vva)
Expand All @@ -41,6 +44,7 @@ def download_and_package_files!
expiration: SECONDS_TO_AUTO_UNLOCK)
s.lock(SECONDS_TO_AUTO_UNLOCK) do
return if pending?

update(fetched_files_status: :pending)
end

Expand Down Expand Up @@ -164,8 +168,13 @@ def requested_zip_at

def update_veteran_info
return unless veteran

update(veteran_first_name: veteran.first_name || "",
veteran_last_name: veteran.last_name || "",
veteran_last_four_ssn: veteran.last_four_ssn || "")
end

def sensitivity_checker
@sensitivity_checker ||= SensitivityChecker.new
end
end
1 change: 1 addition & 0 deletions app/models/manifest_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class ManifestSource < ApplicationRecord

def start!
return if current? || processing?

V2::DownloadManifestJob.perform_later(self, RequestStore[:current_user])
rescue StandardError
update(status: :initialized)
Expand Down
Loading
Loading