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

[EC-293] Fix device verification get settings so the state is correct #2094

Merged
merged 1 commit into from
Jul 5, 2022

Conversation

fedemkr
Copy link
Member

@fedemkr fedemkr commented Jul 4, 2022

Type of change

- [X] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

Fix device verification GetDeviceVerificationSettings so that it returns the correct state when the global setting is off.

Context:
Currently, when the global setting is off the web is showing the Device Verification checkbox On for users given that that value is obtained directly from the UnknownDeviceVerificationEnabled column value of the User table. So, instead the checkbox should be off in that case. Therefore with this change the checkbox gets directly the value of the column only when they can edit the value, otherwise it will be off.

Code changes

  • TwoFactorController.cs: Now it returns the correct state on GetDeviceVerificationSettings. When the user can't edit the device verification setting, then it should return false on UnknownDeviceVerificationEnabled parameter.

Before you submit

- [X] I have checked for formatting errors (`dotnet format --verify-no-changes`) (required)
- [ ] If making database changes - I have also updated Entity Framework queries and/or migrations
- [ ] I have added **unit tests** where it makes sense to do so (encouraged but not required)
- [ ] This change requires a **documentation update** (notify the documentation team)
- [ ] This change has particular **deployment requirements** (notify the DevOps team)

@fedemkr fedemkr requested a review from a team July 4, 2022 18:52
Copy link
Member

@vincentsalucci vincentsalucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build is failing.

Also might be nice to add a comment about why this change is necessary - to me its not apparent based only on the variable names/model constructor.

@fedemkr
Copy link
Member Author

fedemkr commented Jul 5, 2022

@vincentsalucci I think the build failed for a timing issue or something like that, because AFAIK I didn't touch anything about the test that's failing (re-running failed jobs now). Aside from that, I've added more context on the objective to further explain why is this necessary. Let me know if that's fine or further explanation is needed

@fedemkr fedemkr merged commit 580987f into master Jul 5, 2022
@fedemkr fedemkr deleted the EC-293-fix-device-verification-status branch July 5, 2022 21:44
BrandonM-Bitwarden added a commit that referenced this pull request Jul 26, 2022
* [SG-72] Sync changed email address with stripe (#2042)

* sync changed email address with strip

* sync changed email address with strip

* fixed formatting

* throw exception if not successful

* Added revert if stripe sync fails

* Added revert if stripe sync fails

* Added revert if stripe sync fails

* created stripe sync service

* fixed lint issue

* reverted to use stripe exception message

* added null checks to customer id and email address

* added braces

* removed empty email

* EC-262 - implement org user deactivated flag and behavior server (#2050)

* SM-47 - Add Disabled status to enum + schema

* SM-47 - Enable and disable sprocs and repositories

* SM-47 - Organization service enble/disable user

* SM-47 - Fix lint errors

* SM-47 - add disable/enable endpoints to API

* SM-47 - Add bulk operations for enable/disable

* SM-47 - Fix linting errors, one of these days I'll do this first

* SM-47 - Codesense fix DRY warnings

* EC-262 - Code review changes, async cleanup

* EC-262 - Fix build issues, async refs

* EC-262 - Update controller param types

* EC-262 - Ensure mutable state is correct

* EC-262 - rename disabled to deactivated

* [EC-243] Grant premium status when member accepts org invite (#2043)

* EC-262 - add missing validation on deactivate (#2064)

* [PS-293] Update admin portal to use the new version.json (#2006)

* PS-293: Get latest version no comes from GitHub instead of DockerHub.

* PS-293: format fixes

* PS-293: code refactor and clean up

* PS-293: deserialization to class, argument typification.

* PS-293: formating fix

* PS-293: Moved ProjectType to HomeController

* PS-293: updated version endpoint to CDN

* PS-293: Update endpoint to CF protected

* [PS-721] Left align all email template text (#2033)

Make all email template text left-aligned, excluding call-to-actions buttons which should remain centered

In the emails needing updates, remove font styling from HTML tags other than <td> and <p>

Add an additional margin above and below each call-to-action button

For emails that include only the “ignore” warning below the call-to-action button move the warning up, so the button is the last item displayed

Fix the unit test that allows developers to locally generate test emails

* Bumped version to 2022.5.2 (#2067)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* [SG-378] Get and send collectionIds when a cipher is updated (#2066)

* Get and send collectionIds when a cipher is updated

* Make Put method parameters Guids instead of strings

* Check for ascii-only in entire local part of emails (#2072)

* Fix OrganizationConnection Update (#2071)

* Force CloudOrganizationId to be read only

* Fix tests

* [PS-794] Fix password reset email templates email format (#2068)

* Fix password reset email templates email format

* [PS-40] Upgrade to .NET 6 (#2056)

* Bump to .NET 6

* Update Docker images

* Update docs

* Update workflow for linter

* Add all common versions to props file

* Update tools manifest

* Update csproj files

* Update packages.lock.json files

* Switch to setup-dotnet

* Remove msbuild

* Fix deps breaking changes

* Manually install msbuild

* Use msbuild for build

* Fix verbosity switch

* Remove unused exceptions

* Address linter feedback

* Make Obsolete warnings suggestions for now.

* Force Evaluate

* Format on tests

* Run formatting again.

* Use windows 2022

* force evaluate

* Fix restore

* Fix linter

* Skip test

* Update Directory.Build.props

Co-authored-by: Matt Gibson <mgibson@bitwarden.com>

* Address PR feedback

* Add IntegationTest for Rate limiter

* Fix test

* Reenable test

* Reorder test

* Skip test again

* Add tracking link

* Update .github/workflows/build.yml

Co-authored-by: Micaiah Martin <77340197+mimartin12@users.noreply.github.com>

Co-authored-by: Matt Gibson <mgibson@bitwarden.com>
Co-authored-by: Micaiah Martin <77340197+mimartin12@users.noreply.github.com>

* [fix] Only cancel premium subscriptions after failed payments (#2075)

* [fix] Payment Failed webhook fix (#2076)

* Fix VSCode Launch file (#2077)

* Fix launch.json

* Also change pull request template

* [SG-357] Update email text to reflect EUVR updates (#2073)

* fix: made text changes

* chore: html changes

* Address Analyzer Warnings (#2078)

* Address potential errors

* Add tests

* Add clarity

* Run formatting

* Bump version to 2022.6.0 (#2088)

* Bumped version to 2022.6.0

* manually bumping version since automation is broken

* remove the newline at the end

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Joseph Flinn <joseph.s.flinn@gmail.com>

* Update the Release Version Check (#2089)

* Add Seats to Org note (#2086)

* Turn On `ImplicitUsings` (#2079)

* Turn on ImplicitUsings

* Fix formatting

* Run linter

* Create new file when adding license file and updating (#2092)

* Add Swagger generation for Identity (#2058)

* Fix failing tests (#2095)

* EC-293 Fix device verification state when getting its settings (#2094)

* Bumped version to 2022.6.1 (#2102)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* made the get plans endpoint anonymous (#2107)

* [SG-199] Move MP hint to MP change form (#2080)

* chore: backend changes

* fixed: test

* fix: lint

* Fixing missed email template (#2099)

* Bump version to 2022.6.2 (#2111)

* Bumped version to 2022.6.2

* manually bump the version in server

* fixing the newline at the end of the file

* Revert "fixing the newline at the end of the file"

This reverts commit 805e0ce.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Joseph Flinn <joseph.s.flinn@gmail.com>

* [EC-309] Change Device Verification default global setting value to false (#2106)

* EC-309 Change device verification default global setting value to false

* Update src/Core/Settings/GlobalSettings.cs

Co-authored-by: Matt Gibson <mgibson@bitwarden.com>

Co-authored-by: Matt Gibson <mgibson@bitwarden.com>

* Require lint job before building artifacts (#2109)

* Update GlobalSettings.cs (#2112)

* Update Version Bump action hash (#2115)

* [EC-284] Prevent duplicate organization invites (#2113)

* prevent duplicate organization invites with test

* formatting

* Remove tagging Docker images latest on release (#2098)

* [feat] Allow CS to perform bulk actions on Stripe subscriptions from the Admin portal (#2116)

* [feat] Allow CS to perform bulk actions on Stripe subscriptions from the Admin portal

* [fix] An unrelated lint error

* [EC-261] SCIM (#2105)

* scim project stub

* some scim models and v2 controllers

* implement some v2 scim endpoints

* fix spacing

* api key auth

* EC-261 - SCIM Org API Key and connection type config

* EC-261 - Fix lint errors/formatting

* updates for okta implementation testing

* fix var ref

* updates from testing with Okta

* implement scim context via provider parsing

* support single and list of ids for add/remove groups

* log ops not handled

* touch up scim context

* group list filtering

* EC-261 - Additional SCIM provider types

* EC-265 - UseScim flag and license update

* EC-265 - SCIM provider type of default (0)

* EC-265 - Add Scim URL and update connection validation

* EC-265 - Model validation and cleanup for SCIM keys

* implement scim org connection

* EC-265 - Ensure ServiceUrl is not persisted to DB

* EC-265 - Exclude provider type from DB if not configured

* EC-261 - EF Migrations for SCIM

* add docker builds for scim

* EC-261 - Fix failing permissions tests

* EC-261 - Fix unit tests and pgsql migrations

* Formatting fixes from linter

* EC-265 - Remove service URL from scim config

* EC-265 - Fix unit tests, removed wayward validation

* EC-265 - Require self-hosted for billing sync org conn

* EC-265 - Fix formatting issues - whitespace

* EC-261 - PR feedback and cleanup

* scim constants rename

* no scim settings right now

* update project name

* delete package lock

* update appsettings configs for scim

* use default scim provider for context

Co-authored-by: Kyle Spearrin <kyle.spearrin@gmail.com>

* Add version change check in the version bump workflow (#2118)

* Fix flaky test (#2121)

* [EC-307] Fresh desk custom fields integration (#2114)

* Using correct ILogger on FreshdeskController

* Submitting custom fields to Freshdesk

* Set up FreshdeskController to use IHttpClientFactory

* Added unit test for FreshdeskController

* Moved ControllerCustomizeAttribute and ControllerCustomization to Common

* Modified FreshdeskController to use FreshdeskWebhookModel; Edited unit tests to use AutoFixture

* [EC-315] Record user IP and device type for OrgUser and ProviderUser events (#2119)

* update OrgUserDetailsView to include PlanType and other sponsorship parameters previously removed (#2122)

* [SM-82] Add HttpController Attribute to protect secrets manager controllers during development (#2117)

* Adding development only attribute for sm API

* dotnet format changes

* Swapping attribute name to SecretsManager

* Add migration script to rebuild OrganizationView (#2127)

* Add SCIM image build and publish (#2125)

* lowercase op string comparisons (#2129)

* [PS-93] Distributed Ip rate limiting (#2060)

* Upgrade AspNetCoreRateLimiter and enable redis distributed cache for rate limiting.

- Upgrades AspNetCoreRateLimiter to 4.0.2, which required updating NewtonSoft.Json to 13.0.1.
- Replaces Microsoft.Extensions.Caching.Redis with Microsoft.Extensions.Caching.StackExchangeRedis as the original was deprecated and conflicted with the latest AspNetCoreRateLimiter
- Adds startup task to Program.cs for Api/Identity projects to support AspNetCoreRateLimiters breaking changes for seeding its stores.
- Adds a Redis connection string option to GlobalSettings

Signed-off-by: Shane Melton <smelton@bitwarden.com>

* Cleanup Redis distributed cache registration

- Add new AddDistributedCache service collection extension to add either a Memory or Redis distributed cache.
- Remove distributed cache registration from Identity service collection extension.
- Add IpRateLimitSeedStartupService.cs to run at application startup to seed the Ip rate limiting policies.

Signed-off-by: Shane Melton <smelton@bitwarden.com>

* Add caching configuration to SSO Startup.cs

Signed-off-by: Shane Melton <smelton@bitwarden.com>

* Add ProjectName as an instance name for Redis options

Signed-off-by: Shane Melton <smelton@bitwarden.com>

* Use distributed cache in CustomIpRateLimitMiddleware.cs

Signed-off-by: Shane Melton <smelton@bitwarden.com>

* Undo changes to Program.cs and launchSettings.json

* Move new service collection extensions to SharedWeb

* Upgrade Caching.StackExchangeRedis package to v6

* Cleanup and fix leftover merge conflicts

* Remove use of Newtonsoft.Json in distributed cache extensions

* Cleanup more formatting

* Fix formatting

* Fix startup issue caused by merge and fix integration test

Signed-off-by: Shane Melton <smelton@bitwarden.com>

* Linting fix

Signed-off-by: Shane Melton <smelton@bitwarden.com>

* Update 'Dry Run' path in Release workflow (#2124)

* Configure EventsProcessor to use Azurite for local dev (#2120)

* Update restore/revoke error message wording (#2126)

* Re-evaluate lock files to ensure they match project dependencies (#2132)

* Update workflows for SCIM support (#2131)

* [EC-336] Fix invalid user invites when invited via SCIM (#2135)

* Fix invalid user invites when invited via SCIM

* Fix linting

* Set UseScim flag for new organizations (#2134)

* Update workflows for SCIM support (#2133)

* [SG 475] Fix error thrown when changing payment method (#2137)

* Add null check for sources

* Add expand to get customer sources

* SCIM: Associate users to group on PUT/POST (#2139)

* associate users to group on PUT/POST

* fix logic

* [EC-338] Update SCIM code naming conventions (revoked/restore) (#2140)

* Keep old endpoints but mark as deprecated
* Do not change existing sproc naming

* [EC-92] Add organization vault export to event logs (#2128)

* Added nullable OrganizationId to EventModel

* Added EventType Organization_ClientExportedVault

* Updated CollectController to save the event Organization_ClientExportedVault

* Added OrganizationExportResponseModel to encapsulate Organization Export data

* Added OrganizationExportController to have a single endpoint for Organization vault export

* Added method GetOrganizationCollections to ICollectionService to get collections for an organization

* Added GetOrganizationCiphers to ICipherService to get ciphers for an organization

* Updated controllers to use new methods in ICollectionService and ICipherService

* [ENG-71] Updated release job to create and update Github deployment for Jira integration (#2141)

* [ENG-71] updated release job to have Github deployment

* [ENG-71] Updated to use commit instead of v2.

* [ENG-71] Updated to track each server deployment.

Co-authored-by: Todd Martin <>

Co-authored-by: Gbubemi Smith <gsmithwalter@gmail.com>
Co-authored-by: Chad Scharf <3904944+cscharf@users.noreply.github.com>
Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
Co-authored-by: André Filipe da Silva Bispo <andrefsbispo@hotmail.com>
Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Robyn MacCallum <robyntmaccallum@gmail.com>
Co-authored-by: Matt Gibson <mgibson@bitwarden.com>
Co-authored-by: Justin Baur <136baur@gmail.com>
Co-authored-by: Micaiah Martin <77340197+mimartin12@users.noreply.github.com>
Co-authored-by: Addison Beck <addisonbeck1@gmail.com>
Co-authored-by: Joseph Flinn <joseph.s.flinn@gmail.com>
Co-authored-by: Joseph Flinn <58369717+joseph-flinn@users.noreply.github.com>
Co-authored-by: Oscar Hinton <Hinton@users.noreply.github.com>
Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>
Co-authored-by: Vince Grassia <593223+vgrassia@users.noreply.github.com>
Co-authored-by: Jake Fink <jfink@bitwarden.com>
Co-authored-by: Michał Chęciński <michal.checinski@outlook.com>
Co-authored-by: Kyle Spearrin <kyle.spearrin@gmail.com>
Co-authored-by: Rui Tomé <108268980+r-tome@users.noreply.github.com>
Co-authored-by: Kyle Spearrin <kspearrin@users.noreply.github.com>
Co-authored-by: Shane Melton <shanemeltonapps@gmail.com>
Co-authored-by: Todd Martin <106564991+trmartin4@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants