Deploy stages/rc-2018-06-22 to int#2257
Merged
davemcorwin merged 67 commits intostages/intfrom Jun 19, 2018
Merged
Conversation
… (GA and NR) **Why**: Password reset tokens on the url for the password reset page are leaked to third party sites. **How**: Implement redirection from the url with the token after validation to the url without it while saving the token in session.
**Why**: A user should be aware of sensitive account changes **How**: Create a new new_personal_key event when a user changes their personal key.
**Why**: So that hub will be installed if it isn't as expected
**Why**: So that we can start migrating personal keys to a system that works like passwords
…il address **Why**: We strip and downcase emails upon account creation but we do not do the same when passing emails to the rate limiter (rack attack). **How**: Strip and downcase all emails before passing it to rack attack. Write a spec that tests for padding and mixed case.
…n-login LG-351 Rate limiting does not work as expected in all cases of an email address
…-new-personal-key LG-320 Account History should log when their personal key is changed
…ation code **Why**: An attacker can abuse the SMS system and spam users **How**: Create an atomic increment on OtpRequestsTracker. Increment count before checking rate limit. Don't memoize the OtpRequestsTracker returned from the update. (3 fixes total). Testing will need to be done with Burp's repeater to hit idp with parallel requests.
**Why**: When a user attempts to confirm a personal key and mistypes the key, they receive an error and the continue button is disabled. **How**: The logic for form validation is setting the input box to invalid after a bad key is entered and the form remains invalid even after changing the input (hence why the button stays disabled). To have it behave as before we can simply remove the validation check that was added for enabling the button.
…ding-sms LG-311 Prevent bypassing account lockout when sending SMS verification codes
…ter-typo LG-315 Can't submit personal key after typo
**Why**: To fix an XSS issue in previous version
**Why**: We are moving the salt/cost attributes into the ciphertexts for pii encryption and password and decoupling password verification from Pii encryption. This is the first step in this process. Up next is moving these into the password digest. Then we can drop these columns.
**Why**: To give users more choice during account creation. The current options are: SMS, Voice, Authentication app. PIV/CAC will be added later in the menu, although it is already available if you know the URL.
…eakage LG-259 Password reset tokens should not leak to 3rd party sites (GA and NR)
**Why**: User could lose account access if password reset link sent to incorrect or inaccessible address **How**: Remove the Devise mail helper initializer added to keep unencrypted email addresses from log files, now superfluous since all job log entries have standardized output via our custom `ActiveJob::Logging::LogSubscriber`
**Why**: User could lose account access if password reset link sent to incorrect or inaccessible address **How**: Remove the Devise mail helper initializer added to keep unencrypted email addresses from log files, now superfluous since all job log entries have standardized output via our custom `ActiveJob::Logging::LogSubscriber`
LG-162 - Offer multiple 2FA options during account creation
…-email-reset LG-283 Fix password reset links sent to unconfirmed email address
Previous versions of `secure_headers` obscured a bug that should have caused the spec on line 79 of `spec/features/users/sign_up_spec.rb` to fail. Before version 6.0.0, the gem kept track of which CSP directives weren't supported in which browsers, and parsed the user agent and overrode the app's CSP config to remove any directives that it thought were not supported. With the user agent parsing code, the `HeadlessChrome` User Agent was being interpreted by the `secure_headers` gem as a browser that does not support the `form-action` directive, which allowed the aforementioned spec to pass (because the `form-action` directive was removed from the CSP headers). I verified this using the middleware linked to in this [blog post](https://about.gitlab.com/2017/12/19/moving-to-headless-chrome/), which allows us to inspect the response headers (Selenium doesn't have support for `page.response_headers`). In the latest version of `secure_headers`, the `form-action` directive is not removed (which is a good thing), and the reason the test fails is because this is a JS test, and we have configured all JS tests to use `127.0.0.1` as the domain name, but in the last part of the test, when the user clicks on the button in the modal to cancel account creation and delete their account, it tries to go to `http://www.example.com/sign_up/start`. This is because the `UsersController` determines where to redirect based on the `cancel_link_url` of the `decorated_session`, which in this case is the `ServiceProviderDecoratedSession`, whose `cancel_link_url` was constructed using `Rails.application.routes.url_helpers`, which, for some reason, ignores the `default_url_options` defined in `ApplicationController` and defaults the host to `www.example.com`, and since the `form-action` directive doesn't allow `example.com`, the test fails to redirect. Note that the defaulting to `www.example.com` only happens in the test environment. I verified this works fine in production. I verified this by replacing `redirect_to url_after_cancellation` with `redirect_to sign_up_start_url` in `UsersController`, which allowed the test to pass. So then I tried passing in the `host` parameter to `sign_up_start_url` in the decorator, but that didn't work because Capybara uses a port with `127.0.0.1`, but the `sign_up_start_url` in the decorator doesn't include the port, and so the `form-action` directive will not allow this redirect. The port is random each time the test is run, so there's no obvious way to set the port, and `Capybara.always_include_port = false` didn't work. So then I looked at the decorator class some more, and noticed the `view_context` that gets passed to it and it struck me: we should be using `view_context.sign_up_start_url` instead of including `Rails.application.routes.url_helpers`, and 🎉!
**Why**: So we can make changes to existing SP's or add new ones at anytime and not have it coupled to the release process. **How**: Treat our SP configurations and assets as data. Allow urls for public certs and logos and whitelist the static site or our github repo as a CMS. Create a remote settings table that caches the current version of agencies.yml and service_providers.yml seeder files and update the seeders to conditionally use the remote content. Provide a rake task that allows us to update, delete, list, and view the current remote settings.
Update secure_headers from 3.7.3 to 6.0.0
**Why**: Our Rubocop config mistakenly excluded the entire `spec` directory. We want our specs to adhere to our Rubocop rules, and where necessary, we can exclude individual specs.
Update .rubocop.yml and fix offenses
**Why**: USSS has issued a new cert. **How**: Replace the old cert.
Why: A user should be aware of sensitive account changes How: Create a new password change event when a user resets their password.
…for-password-reset LG-368 Account History should log when password changed from reset
…vice LG-367 Update certificate for Secret Service PIX SP in production
**Why**: Some of our users may only have a piv/cac available. We want them able to create accounts with piv/cac as their primary 2fa option. **How**: We add piv/cac as an option in the list of 2fa options during account creation.
…ng-account-creation [LG-358] Allow piv/cac as 2fa for account creation
…creation LG-356 Add help text to the account creation screen for SAM
…imeouts **Why**: To be able to diagnose timeouts in production **How**: Monkeypatch the newrelic gem and override the code to truncate backtraces. The length of backtraces is not currently configurable but may be in the future.
LG-353 Add sufficient request tracing to be able to better diagnose timeouts
**Why**: To use the latest and greatest. Note that you will need to delete and regenerate your login.gov GPG key for the tests to pass on your local machine: ``` gpg --list-secret-keys ``` Find the key with uid `[ultimate] login dot gov (development only)` and copy and paste its 40-character key at the end of this command: ``` gpg --delete-secret-keys ``` Then run `make setup`
In Ruby 2.4, `String#match?`, `Regexp#match?` and `Symbol#match?` have been added. The methods are faster than `match`. Because the methods avoid creating a `MatchData` object or saving backref. So, when `MatchData` is not used, use `match?` instead of `match` or `=~`. Reference: http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Performance/RegexpMatch
**Why**: PIV/CAC users are required to configure a backup phone number so that they can sign in/recover their account if they no longer have access to their PIV/CAC card.
LG-145 Update Ruby from 2.3.5 to 2.5.1
LG-322 Prompt PIV/CAC user to set up phone number
**Why**: So that if a user successfully verifies on their last attempt, we don't redirect to the failure screen from the success screen.
**Why**: We don't want the nonce discoverable by JavaScript. **How**: We use an endpoint to create a redirect to the piv/cac service. We create a new nonce when we send the redirect.
…r-than-link [LG-257] Use redirection to hide nonce from HTML
**Why**: SAM wants to add a redirect uri and a new SP that they can use to test prod from UAT. **How**: Update service_providers.yml
LG-372 Update SAM SP in production
**Why**: We don't want everyone being able to take a token and inspect it. We only want the IdP able to do this. **How**: We have several options, including client/server TLS certificates. But adding an HMAC with a shared secret is the lightest weight way to lock down the decoding endpoint.
**Why**: We added the `Encryption` namespace to encapsulate the code related to encryption. Previously this code primarily lived in the `Pii` namespace. This commit moves the underlying encryptor and related classes / modules into the encryption module.
**Why**: Integration testing is complete and they are ready to go to production. **How**: Update service_providers.yml, add cert and logo, create a new agency (DOE)
* Make the generic failure template a partial * Update jurisdiction fail screens
LG-376 Add DOE / Fossil Energy SP to production
**Why**: When implementing the feature that requires PIV/CAC users to set up a backup phone, I reused an existing view but didn't update the link to point back to the account recovery form that's specific to PIV/CAC users. This fixes the link.
Remove constant already initialized warning on MAX_BACKTRACE_FRAMES
LG-373 Use correct 2FA options link for PIV/CAC
**Why**: If someone creates an account and adds their piv/cac and then logs out, on subsequent login, they had the option of adding a phone number when prompted for their piv/cac. This allowed someone with the username and password to bypass the piv/cac second factor and set the phone, which would then be used as the second factor. **How**: Only allow phone setup during account creation or after authenticating with a second factor.
…ication Don't set up 2fa without authenticating first
…pdates LG-309 Allow dynamic service provider updates in production
**Why**: The presenter argument expects an otp_delivery_preference, not a User object.
Fix presenter in Users::PhonesController#update
Contributor
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.