-
Notifications
You must be signed in to change notification settings - Fork 66
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
Add authorization to SM #396
Conversation
@aub Aside from re-running the vcr_cassettes, the diff would suggest that there were more changes. Is this just some weird github issue for diffs on yaml files? |
Was it necessary to re-run cassettes for this change? |
@aub I think this LGTM, but lets not merge it until later in the week. Per conversation in sprint planning. CC: @ayaleloehr |
@saneshark I didn't rerun cassettes (don't know how to), I just copied the session request with some modifications from the Rx cassettes to the SM ones because the specs were failing without the ability to respond to that request. If that's a bad idea I'm happy to fix it. |
Yea, let's not modify the vcr_cassettes. I've set them such that they should only be run for specs in That is to say, a true request spec would use the existing session for all subsequent requests after authentication, fetched from redis, rather than making the additional call. This is what a before(:all) or before(:each) would help to facilitate. You can see what I did for prescriptions_request_spec before(:each)
allow_any_instance_of(RxController).to receive(:client).and_return(authenticated_client)
end |
Can this get merged once the frontend readds the login? @ayaleloehr |
Looks like @saneshark and @aub need to work out their differences on VCR cassettes before this gets merged. |
I'm taking care of that now. Should be ready shortly. |
@aub can you do |
@saneshark ok I rolled back the cassettes changes. They still show a diff, but it's just whitespace changes from my editor. |
@aub LGTM, other than the rubocop complaint. Which editor just out of curiosity? atom does this for EOL for me, but hadn't noticed it with inline whitespace. |
@saneshark it's Atom, and the whitespace was EOL. |
Good to merge from my side, as we just merged the front end login as well. |
It's going to pass checks here in a sec, and then I'll get it merged. Thanks all |
@aub merging, because I need your changes to user factory for doing the same thing to prescriptions. |
* mvi cert and key are figaro required keys, but should not be read in dev (#399) * mvi cert and key are figaro required keys, but faked values should return nil * added initializer to load mvi cert and key * removing unused vcr_cassettes (#412) * Changed default rx sort to prescription_name (#403) * Changed default rx sort to prescription_name * Doh. spec test heading * Prune applications that were processed > 1 month ago (#364) * EVSS: Queue requests for decision (#407) * EVSS: Queue requests for decision * Spec fixes * Finish refactor of common client part 2 (#411) * adding secure messaging * adding spec to improve test coverage issue * stylistic improvements * refactoring prescriptions * rubocop style change * removing old rx parser * removing unused artifacts * adding new prescriptions factory * Allow Education Forms to be sent to a different facility (#400) * Allow Education Forms to be sent to a different facility * PR Feedback * Swallow exception on MVI missing (#413) * swallow exception on MVI missing * remove cruft * Add authorization to SM (#396) * add authorization to SM * revert cassettes * revert cassettes * fix specs without cassettes * fix one more * rubocop * fix coverage * Log validation errors in sentry (#404) * send validation error to sentry * test raven capture exception * return the user (#418) * EVSS: Remove intent to file (#414) Wasn't being used by us, so remove it. * re-enabling auth for rx (#417) * re-enabling auth for rx * fixing stupid * DRY up MHV concerns * add in the concerns * bypass simplecov stupidness * fixing issue from code review * Instrument sidekiq (#410) * Instrument sidekiq * Add sidekiq stats job spec * Cron format for stats job prevents duplicate runs * Adds notes on usage to sidekiq scheduler * EVSS: Add an old claim reaper (#419) * EVSS: Add an old claim reaper We don't have the ATO to store claims indefinitely, so we need to delete old ones periodically. * Update name and schedule * Return empty vaprofile if mvi nil (#422) * updating specs for nil va_profile * va_profile return as null if mvi is down or record not found * don't need profile in user.mvi nil spec * rubocop * Update mock mvi users (#354) * Changes to reflect updated GeoServices VHA attributes (#425) * finish daily report job (#392) * add govdelivery server to app yml * use govdelivery server in staging email method * use govdelivery server in config helper * fix server name * add year to date report to scheduler * perform doesnt require argument * fix statds for spring, add govdelivery server to figaro * add govdelivery server to travis * add va stakeholders * lint * rename report mailer * rename constants * Adding MHV accounts for some mock users (#427) * flush specific redis namespaces via rake (#421) * flush specific redis namespaces via rake * minor fix * allow rake task to invoke environment for models * Application Controller: Send exception to Sentry/Raven (#424) * EVSS: Add a new field/serializer for the list view (#423) * EVSS: Add a new field/serializer for the list view * Add some tests * Add NotImplementedError * Upload nil tracked item (#429) * Increase timeout for EVSS * Remove breakers for testing EVSS * Add some nocov to get up to coverage while breakers are disabled * Omit tracked item id if not present * Set document filename to sanitized name * Clear workers before each spec * Thread pagination (#431) * Removed pagination from threads * new schema for thread * added optional draft id in creates (#432) * added optional draft id in creates * Added draft id to sm documentation * Allow a blank gender in User model validation (#428) * allow a blank gender in User model validation * ask alastair if this is preferable * gender is more likely to be nil than blank. added specific test for it * per alastairs recommendation, put this check back in * Fix folder create (#441) * Update VA stakeholders list (#433) * update va stakeholders * lint * handle routing errors with JSONAPI (#426) * handle routing errors with JSONAPI * routing errors fix * commented out action_missing method, added stylistic improvements * EVSS: Add receivedFromOthersList to events timeline (#436) Fixes https://github.com/department-of-veterans-affairs/sunsets-team/issues/183 * update va_profile, return null for mvi down, 'not found' for no record (#438) * added mvi status to user va_profile * EVSS: Return sync status in json meta (#439) fixes https://github.com/department-of-veterans-affairs/sunsets-team/issues/193 * EVSS: Add other documents to events timeline (#444) * EVSS: Add other documents to events timeline Refs https://github.com/department-of-veterans-affairs/sunsets-team/issues/197 * Add date to objects * Clearer spec * Add SLO Logout (#443) * start putting together SLO * remove junk, add junk, fix signing * finalize SLO * set the logout relay * less dopiness * delete the user too * do not delete the user * implement suggestions from Bill * change the action name * Use EVSS ID for disability claims lookup (#446) * Use EVSS ID for disability claims lookup * Add comment on ID aliasing * MHV Activity Logging Client (#442) * initial commit of mhv activity client * implementing the client for mhv audit logging * adding the breakers service to breakers initializer * dryed up the common issues and addressed comments from code review * remove unnecessary comma * change spool filenames (#445) * EVSS: Set a flag when a user requests a decision (#449) * EVSS: Set a flag when a user requests a decision Solves an issue where the job might not be processed for a while * Rubocop * simplify spec * Sidekiq stats job on critical queue (#435) * Make mvi processing code env var i451 (#453) * added mvi service handling for 200 from vaafi but 500 from mvi (#454) * stop using a singleton for SAML::SettingsService (#447) * stop using a singleton for SAML::SettingsService * do the whole SSL thing * fix spec * MHV Login/Logout facility Integration (#452) * adding the logout logging facility for mhv as an integration, specs to follow * remove unused redis_store change * only do it mhv_correlation_id exists * allow specs to pass but still need to add specs to test service * get test coverage to 100% * addressing comments from @ayal * oops * fixing linter issues * Enable breakers for EVSS services (#457) * Pv facloc pagination (#437) * Error logging on GIS errors * Pagination functional. * Sorting by distance from bbox center * use breakers for MVI (#448)
Only let users access SM if they are LOA3 and have a MHV correlation ID. This pretty much just makes the SM code be in line with the changes @saneshark made for Rx. I made some slight rearrangements to the Rx controller, but the functionality is exactly the same there. This looks like a big change, but almost all of it is in the specs.
@saneshark do we need to temporarily comment this out as you have done with the Rx controller?