Copied from https://github.com/department-of-veterans-affairs/prescriptions-api/issues/67

Lower priority this week than SM work.

The DevOps team needs brief documentation around services the application integrates with so we can better configure our monitoring. This should include at least a brief description of the service, what components of the vets-api depend on that service, expected potential failures, what behaviors to expect from our app during failures, and if available, how to test the health of the service independently of application actions.

This documentation will also be useful for incident response.

Both with and without attachments

Marc and I drew this up on the board yesterday and he will provide more details on how filtering will work in the API documentation he is writing in #87

See https://github.com/department-of-veterans-affairs/kudos-team/blob/master/FacilityLocator/Facility%20Data%20Samples.md for a description of the underlying data.
See #134 for related discussion of VA health facility data.

Like #134 we need to define the output of REST queries for choice act (non-VA provider) data.

Whereas the VA facility GIS data returns facilities (i.e. hospitals/clinic) and their services, the Choice act GIS data returns individual providers (i.e doctors). The data is de-normalized, so a given doctor may have multiple records for each of the specialty they provide at a location, and may have multiple records for each of the locations that they work from. So we need to do some level of aggregation - at least combining all of a doctor's specialties at a given location, and possibly combining all providers at a given location so we can display a map pin.

It does not appear that there is any stable unique identifier that lets us aggregate all the providers and services at a single location (i.e. at a map pin). The UniqueProviderCode identifies a doctor but may give multiple locations. So it may not make sense to provide a "get single record by ID" API operation for getting details - all operations on this may effectively be queries by lat/long. But we need to analyze the data to see how accurate the lat/long data is.

Cache MVI responses in redis with a configurable TTL.

Normally webmock would catch any external requests, but if they occur in an initializer prior to WebMock.disable_net_connect! they will actually be made.

https://github.com/department-of-veterans-affairs/vets-api/blob/master/config/initializers/saml_settings.rb is one such example.

If the IDP is down, this will result in a build breaking or test environments failing, we need to have a more robust mechanism for handling these issues and we should always make sure they happen at an application runtime level, not at initialization.

One solution would be to move the saml_settings into a service class that is invoked at controller level, and have it be redis backed -- the metadata would be loaded from redis first, if it is not available it will make the request and cache it to redis for future requests. We should set the timeout window to something short -- like 5-10 seconds, and we should rescue exception with our own custom exception class to ensure that the error is logged, JSON is returned informing of the issue. In this case, it would likely be a 500 level error.

Another solution would be to hardcode the metadata, if this is not subject to change that is. We could then just load it as an ENV var.

right now our config to allow CORS is split between nginx and Rails. I think it will be easier to manage if we just use https://github.com/cyu/rack-cors

Similar to what @saneshark did here: #88

Then update this issue: department-of-veterans-affairs/messaging-fe#40

@robbiethegeek asked me to write up some thoughts on the API architecture and how it relates to AuthN and AuthZ. So here goes...

For AuthN, there are 2 pieces:

1. ID.me session
2. www.vets.gov session

For AuthZ, there are 2 pieces:

1. Delegated Authorization server/generator
2. api.vets.gov authorization consumer/validator

For the API business logic, there are at least 2 pieces:

1. Database access to the "Veteran Profile".
2. Access to the "task queue."

The above is generic architecture that should be true regardless of underlying technology choice. The only axioms are (a) www.vets.gov and api.vets.gov are 2 different origins (b) id.me is providing identity.

AuthN (authentication)

Input: (maybe) User's username/password
Endpoints: Whatever the authnrequest endpoint in id.me is and https://www.vets.gov/auth/saml-acs-url
Output: A session on the www.vet.gov origin with identity info that's valid for the session and used by the authz step below.

For (1) id.me is handling the login session. We should verify how this session behaves. 3 user scenarios matter:

1. User has no session with id.me and a SAML bounce is initiated from vets.gov. Here, user should be prompted from a password and sent back to vets.gov. If vets.gov session is destroyed (eg www.vets.gov cookies cleared)...and user visits id.me, ensure password is repromtped. The easy architecture mistake here is id.me retains the login session so the user just silently gets regarnted a SAML Assertion.
2. User has existing session with id.me (eg, via id.me wallet) and SAML session is initiated from vets.gov. It's unclear what the right behavior here is. By default, I expect the user will just log into vets.gov silently without a password prompt. It's unclear that if we clear the vets.gov session if they'd log out of id.me too. This is the tricky scenario that we need to hammer out with their product team ASAP as it is possibly an architecture level change for their service.
3. User has existing session with www.vets.gov and logs out of www.vets.gov. Expected behavior is if they hit login, they will be reprompted for a password. Again, interactions id.me session properties need to be exercised like in the above.

Note that the ACS URL should be on www.vets.gov and NOT api.vets.gov. This is because your login session that has your identity is best located along with the webapps itself. If you host on api.vets.gov, this gets a bit harder as you are now creating an additional (unnecessary) cross-origin information share for no real reason.

In the simplest setup, https://www.vets.gov/auth/saml-acs-url should set a secure https cookie that holds whatever login or identity info is necessary for the app.

If you want to get more complicated, you could store that info client-side only with HTML5 sessionStorage or localStorage...but in all cases, the information is bound to the www.vets.gov origin and not the api.vets.gov origin which is conceptually cleaner because api.vets.gov remains stateless and because you're not poking holes in the origin security boundary between api.vets.gov and www.vets.gov.

For a login session, I think cookies or localStorage are the best ways to store the info. Using server-side sessions feels overkill and places an unnecessarily scaling dependency on the server having a coherent database across all webserver instances (imagine we get a new east region but TIC still bounces people between E/W every 30 seconds via DNS round-robin...you'd have a crazy hard time keeping the session DB consistent).

sessionStorage is tempting, but not quite right here because of the session forking semantics when a new window is created. If you right-click "open new tab" on a link, the two sessionStorages are now forked and diverge. If you open a new tab and navigate back to www.vets.gov, there is no way to access the sessionStorage since the objective is to give tab isolation meaning the user will behave as if they are logged out. sessionStorage is probably great for transient app state...not so much for login.

For storage of the identity attributes, I would just dump it in a secure cookie unless you have a strong reason to keep the browser from seeing the data...but that's weird since you're basically asking a user not to attack their own PII.

So in summary:

• www.vets.gov creates AuthNRequest to id.me
• id.me returns SAML Assertion with identity attributes to www.vets.gov/auth/saml-acs-url
• an actual app server running on www.vets.gov receives the Assertion, decrypts it, and creates a session on www.vets.gov with the login credentials and identity attributes. This identity attributes should be made available to the javascript context of the react apps but the login session should be protected as usual via secure cookies.
• This login session will then be used in the AuthZ step to generate the delegated authorization for the API server. This is why the login session must exist and be bound to www.vets.gov and not api.vets.gov.

AuthZ (Authorization)

Input: A valid login session, likely in a cookie.
Endpoints: https://www.vets.gov/auth/oauth2
Output: An OAuth2 access token that can be used in the Authorization header for request to api.vets.gov

NOTE: if you do not have api.vets.gov DNS stood-up yet, I highly suggest creating a heroku app that reverse proxies the API server so you can correctly simulate the API server being on a different origin! If you do not do this, you're gonna run into all sorts of fun with rails sessions colliding. Spinning up a reverse proxy hack should cost you like 1/2 a day but ensure you are developing against the right security contraints.

Rant... There's been a lot of argument about JWT vs Oauth2. It doesn't matter which you use, but I think it's silly to use JWT cause the world uses OAuth2, JWT does not solve the same problem, and the OAuth2 detractors are...well...just the same few people who write shrill blogs. We've spent more time arguing about this than it'd take to write up both prototypes. You can wire up an OAuth2 provider in about 1-2 days and the consumer libraries are tested. Just use OAuth2. It'll keep the API easily compatible with mobile ecosystems and frameworks as well which is a longer term goal. Or if there is really strong resistance still (there shouldn't be. stop arguing for JWT. Oauth2 setup is not complicated...the libraries abstracted ita ll), at least stop discussing whether or not to use it and stand up some real authorization server asap. The devs need to learn how to pass these things around and debug authorization issues which is being blocked by pushing off standing up a real one. If there's a question about this...ask how many devs are thinking about CORS problems or writing wrappers for fetch in react? If the answer isn't everyone, then there's learning to be had that's being blocked.
...End-Rant

For the MVP, create 1 "scope" (this is just a string you put in the configs for the server and client libraries) named https://www.vets.gov/oauth/scopes/all or something that is just "all access" for a user. You can create more restricted scopes (eg 'https://www.vets.gov/oauth/scopes/rx-refill', 'https://www.vets.gov/oauth/scopes/secure-messaging') later if there is some meaningful separation. But for now, just one uber access scope makes sense.

After the server is stood up with a configured scope, write it up to a webapp to get an access token after having logged in. Assuming the user has a valid www.vets.gov session and that the authorization server running doorkeeper has access to this session (hint: use the same app server for the saml acs_url as authorization server so they can share the session secret)

Assuming https://github.com/doorkeeper-gem/doorkeeper gem, I think the following is accurate:

1. Client app uses OAuth2 library to generate a authz request for https://www.vets.gov/oauth/scopes/allto https://www.vets.gov/auth/oauth2/authorize
2. https://www.vets.gov/auth/oauth2/authorize server notices "it is a first party auth" (make the controller skip prompting the user to click "authorize this app" if the request is coming from the registered www.vets.gov application... this flow is useful if a non-vets.gov source like a mobile app wants to use api.vets.gov). This controller now becomes just a check that we have a www.vets.gov session and is otherwise a no-op.
3. https://www.vets.gov/auth/oauth2/authorize returns the authorization code (aka refresh token) to the app. This is given to the oauth-2 client library which will at some point do another post that exchanges the authorization token for an actual "access token" that will be passed to `api.vets.gov.
4. Client App has all information needed to create API requests to api.vets.gov. We likely want to create a wrapper around fetch that annotates the access tokens from above onto all API requests in an Authorization: header.

On the api-server side, add in the right authorization checks with doorkeeper to validate the tokens for the target endpoints.

API business logic

Input: POST/GET to api.vets.gov with Oauth2 Authorization token.
Endpoints: api.vets.gov/*
Output: Some JSON response and maybe side-effects to backend systems.

This is the easiest portion. The React app should be making API request with just the access token. api.vets.gov will have to associate in the backend the oauth2 token with the veteran profile. After validation of the access token is completed and the associated profile is found, the authn and authz technologies are done. Now it's just standard business logic checks to do the real authorization.

@patrickvinograd VHA data (which is what we have available via the VA API right now) has services available at facilities. these services look flat in the data returned from the VA's API, however they are actually nested in that some of the services are major categories (service level 1 = SL1) and some are minor categories (Service level 2 = SL2) within major categories. An example of this is that DiagnosticServices is an SL1 while ImagingAndRadiology and LabServices are SL2's that fit under DiagnosticServices.

I believe that instead of returning something like (this is just a subset of what's returned for a facility):

{
    "DiagnosticServices": "YES",
    "ImagingAndRadiology": "YES",
    "LabServices": "NO",
}

We should do something like:

{
    "DiagnosticServices": {
        "ImagingAndRadiology": true,
    }
}

Two changes here:
(1) is that it ImagingAndRadiology is nested under DiagnosticServices, so the front end knows they are associated
(2) is that only the services that are available at the facility show up, rather than having a whole lot of "NO"s in there.
Note, the "true" there at the end still feels weird to me and I'm sure there is a better way of doing, but I just wanted to get this concept overall out there first.

Need to implement all routes and functionality related to attachments

Using this issue to come to agreement about facility locator data format. Limiting discussion for now to VA health facilities. Will file a separate issue for Choice Act provider data, and likely a separate issue for VA benefits and cemetery facilities.

See baseline data available from GIS data source in https://github.com/department-of-veterans-affairs/kudos-team/blob/master/FacilityLocator/Facility%20Data%20Samples.md
See nesting discussion in #126

Here is a baseline object:

{ "attributes":
{"OBJECTID":1510,
"FacilityDataDate":"8-19-2016",
"OutpatientServicesDataDate":1472428800000,
"StationID":539,
"VisnID":20,
"StationNumber":"648A4",
"StationName":"Portland VA Medical Center-Vancouver",
"CommonStationName":"Portland-Vancouver",
"CocClassification":"VA Medical Center (VAMC)",
"CocClassificationAttribute":"Firm",
"Building":null,
"Street":"1601 East 4th Plain Boulevard",
"Suite":null,
"City":"Vancouver",
"State":"WA",
"Zip":"98661",
"Zip4":"3753",
"MainPhone":"360-759-1901 x",
"MainFax":"360-690-0864 x",
"AfterHoursPhone":"360-696-4061 x",
"PatientAdvocatePhone":"503-273-5308 x",
"EnrollmentCoordinatorPhone":"503-273-5069 x",
"PharmacyPhone":"503-273-5183 x",
"Monday":"730AM-430PM",
"Tuesday":"730AM-630PM",
"Wednesday":"730AM-430PM",
"Thursday":"730AM-430PM",
"Friday":"730AM-430PM",
"Saturday":"800AM-1000AM",
"Sunday":"-",
"Latitude":45.63941626,
"Longitude":-122.65528736,
"Audiology":"YES",
"ComplementaryAlternativeMed":"NO",
"DentalServices":"YES",
"DiagnosticServices":"YES",
"ImagingAndRadiology":"YES",
"LabServices":"NO",
"EmergencyDept":"NO",
"EyeCare":"YES",
"MentalHealthCare":"YES",
"OutpatientMHCare":"YES",
"OutpatientSpecMHCare":"YES",
"VocationalAssistance":"YES",
"OutpatientMedicalSpecialty":"NO",
"AllergyAndImmunology":"NO",
"CardiologyCareServices":"NO",
"DermatologyCareServices":"NO",
"Diabetes":"NO",
"Dialysis":"NO",
"Endocrinology":"NO",
"Gastroenterology":"NO",
"Hematology":"NO",
"InfectiousDisease":"NO",
"InternalMedicine":"NO",
"Nephrology":"NO",
"Neurology":"NO",
"Oncology":"NO",
"PulmonaryRespiratoryDisease":"NO",
"Rheumatology":"NO",
"SleepMedicine":"NO",
"OutpatientSurgicalSpecialty":"YES",
"CardiacSurgery":"NO",
"ColoRectalSurgery":"NO",
"ENT":"NO",
"GeneralSurgery":"NO",
"Gynecology":"NO",
"Neurosurgery":"NO",
"Orthopedics":"NO",
"PainManagement":"NO",
"PlasticSurgery":"NO",
"Podiatry":"YES",
"ThoracicSurgery":"NO",
"Urology":"NO",
"VascularSurgery":"NO",
"PrimaryCare":"YES",
"Rehabilitation":"YES",
"UrgentCare":"NO",
"WellnessAndPreventativeCare":"YES"
},
"geometry":{"x":-122.65530186353662,"y":45.63942173397186}
}

Proposed format is as follows:

• Omitting fields: OBJECTID, FacilityDataDate, OutpatientServicesDataDate,CocClassificationAttribute
• The "Latitude,Longitude" fields are returned in preference to the GIS "geometry" fields.
• Key names generally simplified and changed to lowercase.
• "StationID" -> "id"
• "VisnID" -> "visn_id"
• "StationName" -> "name"
• "CocClassification" -> "classification"
• Address fields nested under "address"
• Phone fields nested under "phone"
• Hours fields nested under "hours"
• Services nested hierarchically under "services" as described in #126
• Service type keys are not simplified/lowercased, they are kept exactly as-is in case they are referenced elsewhere.

{ 
  "id":539,
  "visn_id":20,
  "name":"Portland VA Medical Center-Vancouver",
  "classification":"VA Medical Center (VAMC)",
  "address": {
    "building":null,
    "street":"1601 East 4th Plain Boulevard",
    "suite":null,
    "city":"Vancouver",
    "state":"WA",
    "zip":"98661",
    "zip4":"3753"
  },
  "phone": {
    "main":"360-759-1901 x",
    "fax":"360-690-0864 x",
    "after_hours":"360-696-4061 x",
    "patient_advocate":"503-273-5308 x",
    "enrollment_coordinator":"503-273-5069 x",
    "pharmacy":"503-273-5183 x",
  },
  "hours": {
    "Monday":"730AM-430PM",
    "Tuesday":"730AM-630PM",
    "Wednesday":"730AM-430PM",
    "Thursday":"730AM-430PM",
    "Friday":"730AM-430PM",
    "Saturday":"800AM-1000AM",
    "Sunday":"-",
  },
  "services": {
    "Audiology": [],
    "DentalServices": [],
    "DiagnosticServices": ["ImagingAndRadiology"],
    "EyeCare": [],
    "MentalHealthCare": ["OutpatientMHCare", "OutpatientSpecMHCare", "VocationalAssistance"]
    "OutpatientSurgicalSpecialty": [],
    "PrimaryCare": [],
    "Rehabilitation": [],
    "WellnessAndPreventativeCare": []
  },
  "lat":45.63941626,
  "long":-122.65528736
}

Open issues:

• Whether keys "Common Station Name" and "Station Number" are useful pieces of information.

Creating and updating drafts, sending a draft, and replying with a draft.

I believe this will be needed just for /folders/:id/messages on sentDate, subject, and from

Making everything that is a collection into one; will make sorting/filtering easier as well.

Might get slightly complex with deleting drafts potentially?

(assuming we use ActiveJob in rails, backed by something else)
😄 = Resque
🎉 = Sidekiq
❤️ = DelayedJob
👎 = Other / Not ActiveJob

https://app.moqups.com/[email protected]/l32hJ36b/view/page/ac1f8090a
frontend wants to show the address of where the claim will be sent. we need a method anyways to decide spool file region so we can use that to give them the address too.

Traced error to this line in application.rb:
config.middleware.use "OliveBranch::Middleware"

The error:

/Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/actionpack-4.2.7.1/lib/action_dispatch/middleware/stack.rb:112:in `push': can't modify frozen Array (RuntimeError)
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/actionpack-4.2.7.1/lib/action_dispatch/middleware/stack.rb:112:in 'use'
           from /Users/v/Dsva/vets-api/config/application.rb:43:in '<class:Application>'
           from /Users/v/Dsva/vets-api/config/application.rb:18:in '<module:VetsAPI>'
           from /Users/v/Dsva/vets-api/config/application.rb:17:in '<top (required)>'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:274:in 'require'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:274:in 'block in require'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:240:in 'load_dependency'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:274:in 'require'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/railties-4.2.7.1/lib/rails/commands/commands_tasks.rb:141:in 'require_application_and_environment!'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/railties-4.2.7.1/lib/rails/commands/commands_tasks.rb:67:in 'console'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/railties-4.2.7.1/lib/rails/commands/commands_tasks.rb:39:in 'run_command!'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/railties-4.2.7.1/lib/rails/commands.rb:17:in '<top (required)>'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:274:in 'require'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:274:in 'block in require'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:240:in 'load_dependency'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:274:in 'require'
           from /Users/v/Dsva/vets-api/bin/rails:10:in '<top (required)>'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:268:in 'load'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:268:in 'block in load'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:240:in 'load_dependency'
           from /Users/v/.rvm/gems/ruby-2.3.0@sm-api/gems/activesupport-4.2.7.1/lib/active_support/dependencies.rb:268:in 'load'
           from /Users/v/.rvm/rubies/ruby-2.3.0/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in 'require'
           from /Users/v/.rvm/rubies/ruby-2.3.0/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in 'require'
           from -e:1:in '<main>'

Work with devops to place them into a secure credential store.

Currently the correlation ids in an MVI response arrive in an array, sort them into a hash so they can be later looked up by correlation id type.

i'm thinking about adding a uuid column to education_benefits_claims and using that for the confirmation number, thoughts @markolson?

Opening an issue to track when the secure messaging API returns internal server errors (code 500) on staging.

Link for convenience: https://staging.vets.gov/api/v0/messaging/health/folders.

Occurred again at the time of submitting this issue.

In case the SFTP is inaccessible, we need to retire at a later time.

TODO: figure out if we have to process at the same time each day, or if we can submit whenever.

also add spec for edu benefits claim serializer

Similar to either

https://github.com/department-of-veterans-affairs/vets-api/blob/6edef6d030a4e4aba9289d9d4e52411f28552412/docs/api/edu-benefits.md

or

https://github.com/department-of-veterans-affairs/prescriptions-api/blob/master/api-doc.md

Spool files are broken up to group submissions to different regions based on the school the person is applying for. I think we're submitting 4 spool files a day, but need to verify.

In addition to timeouts from #74 , we should evaluate implementing circuit breakers for each of the application's external service integrations.

Our application has the potential to send more traffic to these systems than they may expect, and if they're down or under high load, the ability to back off and allow them to recover could mean improved overall uptime for our systems.

The DevOps team has configured monitoring for high average request latency from this application, and will work to integrate more metrics for individual services in the future. We've noticed average latency over 5 minute intervals is around ~1.5 seconds with considerable variability (up to ~40s and often between 10 and 30s).

To prevent false alarms and to better understand the application health and performance characteristics we should implement explicit timeouts around external services that this application interacts with. This also has the benefit of providing the customer a more consistent experience, since we'll be more explicitly defining the failure case and providing better feedback for things that take longer than expected or may be down.

The values for these timeouts should be based on the SLAs for the corresponding services.

Does the folder have to be empty? If yes, what is the error returned if you try to delete a folder and it isn't empty?