Skip to content

[PM-8029] Icon loading fixes and uri parsing improvements#3232

Closed
1fexd wants to merge 3932 commits intobitwarden:mainfrom
1fexd:uri-parsing
Closed

[PM-8029] Icon loading fixes and uri parsing improvements#3232
1fexd wants to merge 3932 commits intobitwarden:mainfrom
1fexd:uri-parsing

Conversation

@1fexd
Copy link
Copy Markdown

@1fexd 1fexd commented May 9, 2024

Type of change

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

Objective

  • Avoid loading icons when the user has chosen not to display them
  • Remove abitrary TLD restriction for link open / favicon loading check by adding Nager.PublicSuffix
  • Only try to load favicons for valid domains (via publicsuffix)
  • Never attempt to load favicons for .onion / .i2p domains
  • Improve uri parsing util without changing the existing behavior

I've also added some suggestions on what changes to the parsing behavior should be made in my opinion.

Code changes

  • file.ext: Description of what was changed and why

  • src/Core/Core.csproj: Add NuGet package to load publicsuffixlist, add asset

  • src/Core/Pages/Vault/CipherItemViewModel.cs: Don't load favicon if user has disabled favicons

  • src/Core/Utilities/CoreHelpers.cs: Uri parsing improvements with suggested breaking changes

  • src/Core/Utilities/IconImageConverter.cs: Favicon loading fixes and improvements

  • src/Core/Utilities/MauiAssetRuleProvider.cs: Maui asset rule provider

  • src/Core/Utilities/ServiceContainer.cs: Register rule provider

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

djsmith85 and others added 30 commits August 15, 2023 15:32
* Set team-leads-eng as owners for translations

This is needed to Crowdin sync PRs can be merged.

* Add team-tools as owner of the email-forwarders

* Fix unescaped whitespace

* Remove team-leads-eng from owning English resources
Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com>
Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>
…ons (bitwarden#2698)

* PM-3508 Fix Release iPhoneSimulator configuration for iOS / Extensions

* PM-3508 Fix --deep space on watch app references
* [PM-1208] Add Device approval options screen. View model waiting for additional logic to be added.

* [PM-1208] Add device related api endpoint. Add AccoundDecryptOptions model and property to user Account.

* [PM-1208] Add continue button and not you option

* [PM-1379] add DeviceTrustCryptoService with establish trust logic (bitwarden#2535)

* [PM-1379] add DeviceCryptoService with establish trust logic

* PM-1379 update api location and other minor refactors

* pm-1379 fix encoding

* update trusted device keys api call to Put

* [PM-1379] rename DeviceCryptoService to DeviceTrustCryptoService
- refactors to prevent side effects

* [PM-1379] rearrange methods in DeviceTrustCryptoService

* [PM-1379] rearrange methods in abstraction

* [PM-1379] deconstruct tuples

* [PM-1379] remove extra tasks

* [PM-2583] Answer auth request with mp field as null if doesn't have it. (bitwarden#2609)

* [PM-2287][PM-2289][PM-2293] Approval Options (bitwarden#2608)

* [PM-2293] Add AuthRequestType to PasswordlessLoginPage.

* [PM-2293] Add Actions to ApproveWithDevicePage

* [PM-2293] Change screen text based on AuthRequestType

* [PM-2293] Refactor AuthRequestType enum. Add label. Remove unnecessary actions.

* [PM-2293] Change boolean variable expression.

* [PM-2293] Trust device after admin request login.

* code format

* [PM-2287] Add trust device to master password unlock. Change trust device method. Remove email from SSO login page.

* [PM-2293] Fix state variable get set.

* [PM-2287][PM-2289][PM-2293] Rename method

* [PM-1201] Change timeout actions available based on hasMasterPassword (bitwarden#2610)

* [PM-1201] Change timeout actions available based on hasMasterPassword

* [PM-2731] add user key and master key types

* [PM-2713] add new state for new keys and obsolete old ones
- UserKey
- MasterKey
- UserKeyMasterKey (enc UserKey from User Table)

* [PM-271] add UserKey and MasterKey support to crypto service

* [PM-2713] rename key hash to password hash & begin add methods to crypto service

* [PM-2713] continue organizing crypto service

* [PM-2713] more updates to crypto service

* [PM-2713] add new pin methods to state service

* [PM-2713] fix signature of GetUserKeyPin

* [PM-2713] add make user key method to crypto service

* [PM-2713] refresh pin key when setting user key

* [PM-2713] use new MakeMasterKey method

* [PM-2713] add toggle method to crypto service for keys

* [PM-2713] converting calls to new crypto service api

* [PM-2713] add migration for pin on lock screens

* [PM-2713] more conversions to new crypto service api

* [PM-2713] convert cipher service and others to crypto service api

* [PM-2713] More conversions to crypto api

* [PM-2713] use new crypto service api in auth service

* [PM-2713] remove unused cached values in crypto service

* [PM-2713] set decrypt and set user key in login helper

* fix bad merge

* Update crypto service api call to fix build

* [PM-1208] Fix app resource file

* [PM-1208] Fix merge

* [PM-1208] Fix merge

* [PM-2713] optimize async code in crypto service

* [PM-2713] rename password hash to master key hash

* [PM-2713] fix casting issues and pin

* [PM-2713] remove extra comment

* [PM-2713] remove broken casting

* [PM-2297] Login with trusted device (Flow 2) (bitwarden#2623)

* [PM-2297] Add DecryptUserKeyWithDeviceKey method

* [PM-2297] Add methods to DeviceTrustCryptoService update decryption options model

* [PM-2297] Update account decryption options model

* [PM-2297] Fix TrustedDeviceOption and DeviceResponse model. Change StateService device key get set to have default user id

* [PM-2297] Update navigation to decryption options

* [PM-2297] Add missing action navigations to iOS extensions

* [PM-2297] Fix trust device bug/typo

* [PM-2297] Fix model bug

* [PM-2297] Fix state var crash

* [PM-2297] Add trust device login logic to auth service

* [PM-2297] Refactor auth service key connector code

* [PM-2297] Remove reconciledOptions for deviceKey in state service

* [PM-2297] Remove unnecessary user id params

* [PM-2289] [PM-2293] TDE Login with device Admin Request (bitwarden#2642)

* [PM-2713] deconstruct new key pair

* [PM-2713] rename PrivateKey methods to UserPrivateKey on crypto service

* [PM-2713] rename PinLockEnum to PinLockType

* [PM-2713] don't pass user key as param when encrypting

* [PM-2713] rename toggle method, don't reset enc user key

* [PM-2713] pr feedback

* [PM-2713] PR feedback

* [PM-2713] rename get pin lock type method

* [PM-2713] revert feedback for build

* [PM-2713] rename state methods

* [PM-2713] combine makeDataEncKey methods

* [PM-2713] consolidate attachment key creation
- also fix ios files missed during symbol rename

* [PM-2713] replace generic with inherited class

* rename account keys to be more descriptive

* [PM-2713] add auto unlock key to mobile

* [PM-1208] Add TDE flows for new users (bitwarden#2655)

* [PM-1208] Create new user on SSO. Logout if not password is setup or has pending admin auth request.

* [PM-1208] Fix new user UserKey decryption.

* [PM-1208] Add new user continue to vault logic. Auto enrol user on continue.

* [PM-1208] Trust device only if needed

* [PM-1208] Add logic for New User SSO.

* [PM-1208] Add logic for New User SSO (missing file).

* [PM-2713] set user key on set password page

* [PM-2713] set enc user key during kc onboarding

* fix formatting

* [PM-2713] make method async again
- returning null from a task thats not async throws

* [PM-2713] clear service cache when adding new account

* Fix build after merge

* [PM-3313] Fix Android SSO Login (bitwarden#2663)

* [PM-3313] Catch exception on AuthPendingRequest

* [PM-3313] Fix lock timeout action if user doesn't have a master password.

* code format

* [PM-3313] Null email in Approval Options screen (bitwarden#2664)

* [PM-3313] Fix null email in approval options screen

* [PM-3320][PM-3321] Fix labels and UI tweaks (bitwarden#2666)

* [PM-3320] Fix UI copy and remember me default ON.

* [PM-3321] Fix UI on Log in with device screen.

* [PM-3337] Fix admin request deny error (bitwarden#2669)

* [PM-3342] Not you button logs user out. (bitwarden#2672)

* [PM-3319] Check for admin request in Lock page (bitwarden#2668)

* [PM-3319] Ignore admin auth request when choosing mp as decryption option.

* [PM-2289] Change header title based on auth request type (bitwarden#2670)

* [PM-2289] Change header title based on auth request type

* [PM-3333] Check for purged admin auth requests (bitwarden#2671)

* [PM-3333] Check for purged admin auth requests

Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>

---------

Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>

* [PM-3341] Vault Timeout Action not persisted correctly (bitwarden#2673)

* [PM-3341] Fix timeout action change when navigating

* [PM-3357] Fix copy for Login Initiated (bitwarden#2674)

* [PM-3362] Fix auth request approval (bitwarden#2675)

* [PM-3362] Fix auth request approval

* [PM-3362] Add new exception type

* [PM-3102] Update Master password reprompt to be based on MP instead of Key Connector (bitwarden#2653)

* PM-3102 Added check to see if a user has master password set replacing previous usage of key connector.

* PM-3102 Fix formatting

* [PM-2713] Final merge from Key Migration branch to TDE Feature branch (bitwarden#2667)

* [PM-2713] add async to key connector service methods

* [PM-2713] rename ephemeral pin key

* add state for biometric key and accept UserKey instead of string for auto key

* Get UserKey from bio state on unlock

* PM-2713 Fix auto-migrating EncKeyEncrypted into MasterKey encrypted UserKey when requesting DecryptUserKeyWithMasterKeyAsync is called

* renaming bio key and fix build

* PM-3194 Fix biometrics button to be shown on upgrade when no UserKey is present yet

* revert removal of key connector service from auth service

* PM-2713 set user key when using KC

* clear enc user key after migration

* use is true for nullable bool

* PR feedback, refactor kc service

---------

Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>

* Fix app fresh install user login with master password. (bitwarden#2676)

* [PM-3303] Fix biometric login after key migration (bitwarden#2679)

* [PM-3303] Add condition to biometric unlock

* [PM-3381] Fix TDE login 2FA flow (bitwarden#2678)

* [PM-3381] Check for vault lock on 2FA screen

* [PM-3381] Move logic to ViewModel

* [PM-3381] Fix null vm error

* [PM-3379] Fix key rotation on trusted device. (bitwarden#2680)

* [PM-3381] Update login flows (bitwarden#2683)

* [PM-3381] Update login flows

* [PM-3381] Remove _authingWithSso parameter

* PM-3385 Fix MP reprompt item level when no MP hash is stored like logging in with TDE. Also refactor code to be more maintainable (bitwarden#2687)

* PM-3386 Fix MP reprompt / OTP decision to be also based on the master key hash. (bitwarden#2688)

* PM-3450 Fix has master password with mp key hash check (bitwarden#2689)

* [PM-3394] Fix login with device for passwordless approvals (bitwarden#2686)

* set activeUserId to null when logging in a new account
- Also stop the user key from being set in inactive accounts

* get token for login with device if approving device doesn't have master key

* add comment

* simplify logic

* check for route instead of using isAuthenticated
- we don't clear the user id when logging in new account
- this means we can't trust the state service, so we have to base our logic off the route in login with device

* use authenticated auth request for tde login with device

* [PM-3394] Add authingWithSso parameter to LoginPasswordlessRequestPage.

* pr feedback

* [PM-3394] Refactor condition

Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>

---------

Co-authored-by: André Bispo <abispo@bitwarden.com>
Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>

* [PM-3462] Handle force password reset on mobile with TDE (bitwarden#2694)

* [PM-3462] Handle force password reset on mobile with TDE

* [PM-3462] update references to refactored crypto method
- fix kc bug, we were sending private key instead of user key to server
- rename kc service method to be correct

* [PM-3462] Update TwoFactorPage login logic

* [PM-3462] Added pending admin request check to TwoFactorPage

* [PM-3462] Added new exception types for null keys

---------

Co-authored-by: André Bispo <abispo@bitwarden.com>

* [PM-1029] Fix Async suffix in ApiService. Add UserKeyNullExceptions.

* [PM 3513] Fix passwordless 2fa login with device on mobile (bitwarden#2700)

* [PM-3513] Fix 2FA for normal login with device with users without mp

* move _userKey

---------

Co-authored-by: André Bispo <abispo@bitwarden.com>

* clear encrypted pin on logout (bitwarden#2699)

---------

Co-authored-by: André Bispo <abispo@bitwarden.com>
Co-authored-by: Jake Fink <jfink@bitwarden.com>
Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>
* PM-3071 Removed share on save toggle on Send view and now it's done automatically, same for copy after saving from the Share extension

* PM-3071 Fix alignments on Share extension send view
* Update CODEOWNERS

* Update CODEOWNERS
* [PM-3513] Show API error when SSO policy is enforced.
Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com>
* [PM-3545] Fix biometric unlock for autofill

* [PM-3545] Reuse existing method
* fix for TDE pref naming collision

* fix case
Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com>
Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com>
Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com>
…ll. (bitwarden#2713)

* [PM-3543] [PM-3507] Fix password re-prompt when editing and on autofill.
…rden#2724)

* Migration of data from LiteDB to shared pref storage

* tweaks
Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com>
…g SSO (bitwarden#2719)

* [PM-3382] Update mobile client to receive and use SsoEmail2faSessionToken

* [PM-3382] Fix null 2fa email with local email on MP login.
* Adding build steps for .app

* Uploading .app artifact

* Fixing ARCHIVE_PATH variable

* Fixing missing OutputPath

* Fixing Bitwarden .app file name

* Fixing wrong .app location

* Adding Fede's suggestion

* Update .github/workflows/build.yml

Co-authored-by: Vince Grassia <593223+vgrassia@users.noreply.github.com>

---------

Co-authored-by: Vince Grassia <593223+vgrassia@users.noreply.github.com>
Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com>
* Update AppResources.af.resx

* Update AnonAddy references

* Reverted AnonAddy to AddyIo refactor, keeping text and url changes

---------

Co-authored-by: Andre Rosado <arosado@bitwarden.com>
…2723)

* [PM-3606] Fix 2FA for autofill

* [PM-3606] Fix autofill when user doesn't have a login method available.

* [PM-3606] PR fixes

* [PM-3606] Add logout logic to other extension projects

* [PM-3606] Move code to base class.

* Transform into property instead of field

Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>

* Remove double ";"

Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>

* [PM-3606] Fix iOS extension by changing base class of LockPasswordViewController

---------

Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>
andrebispo5 and others added 21 commits April 26, 2024 13:23
…validation on Fido2 flows. Also fixed typo on "privileged" and updated UT (bitwarden#3198)
* [PM-7690] Move UI thread invocation to viewmodel command
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…ng certain exception to bubble up and explode, so move the if internally. (bitwarden#3208)
Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com>
* PM-7877 Added loading dialog when unlocking with PIN

* PM-7877 Added exception logging on unlock with PIN
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…behavior (bitwarden#3230)

* PM-7255 Fix autofill cancelling the request on password autofill because of wrong safeguard

* PM-7255 Clear code no longer used
@1fexd 1fexd requested a review from a team as a code owner May 9, 2024 23:14
@bitwarden-bot
Copy link
Copy Markdown

Thank you for your contribution! We've added this to our internal Community PR board for review.
ID: PM-8029

@bitwarden-bot bitwarden-bot changed the title Icon loading fixes and uri parsing improvements [PM-8029] Icon loading fixes and uri parsing improvements May 9, 2024
@1fexd
Copy link
Copy Markdown
Author

1fexd commented May 10, 2024

public class Tests
{

    /// <summary>
    /// Returns the second and top level domain of the given uri.
    /// Does not support plain hostnames without
    ///
    /// Input => Output examples:
    /// <para>https://bitwarden.com => bitwarden.com</para>
    /// <para>https://login.bitwarden.com:1337 => bitwarden.com</para>
    /// <para>https://sub.login.bitwarden.com:1337 => bitwarden.com</para>
    /// <para>https://localhost:8080 => localhost</para>
    /// <para>localhost => null</para>
    /// <para>bitwarden => null</para>
    /// <para>127.0.0.1 => 127.0.0.1</para>
    /// </summary>
    [Test]
    public async Task Test_GetDomain()
    {
        var coreHelpers = await Create();

        Assert.That(coreHelpers.GetDomain("https://bitwarden.com"), Is.EqualTo("bitwarden.com"));
        Assert.That(coreHelpers.GetDomain("https://login.bitwarden.com:1337"), Is.EqualTo("bitwarden.com"));
        Assert.That(coreHelpers.GetDomain("https://sub.login.bitwarden.com:1337"), Is.EqualTo("bitwarden.com"));
        Assert.That(coreHelpers.GetDomain("https://localhost:8080"), Is.EqualTo("localhost"));
        Assert.That(coreHelpers.GetDomain("localhost"), Is.EqualTo(null));
        Assert.That(coreHelpers.GetDomain("bitwarden"), Is.EqualTo(null));
        Assert.That(coreHelpers.GetDomain("127.0.0.1"), Is.EqualTo("127.0.0.1"));
        Assert.That(coreHelpers.GetDomain("linksheet.local"), Is.EqualTo("linksheet.local"));
    }

    /// <summary>
    /// Returns the host and port of the given uri.
    /// Does not support plain hostnames without
    ///
    /// Input => Output examples:
    /// <para>https://bitwarden.com => bitwarden.com</para>
    /// <para>https://login.bitwarden.com:1337 => login.bitwarden.com:1337</para>
    /// <para>https://sub.login.bitwarden.com:1337 => sub.login.bitwarden.com:1337</para>
    /// <para>https://localhost:8080 => localhost:8080</para>
    /// <para>localhost => null</para>
    /// <para>bitwarden => null</para>
    /// <para>127.0.0.1 => 127.0.0.1</para>
    /// </summary>
    [Test]
    public async Task Test_GetHost()
    {
        var coreHelpers = await Create();

        Assert.That(coreHelpers.GetHost("https://bitwarden.com"), Is.EqualTo("bitwarden.com"));
        Assert.That(coreHelpers.GetHost("https://login.bitwarden.com:1337"), Is.EqualTo("login.bitwarden.com:1337"));
        Assert.That(coreHelpers.GetHost("https://sub.login.bitwarden.com:1337"), Is.EqualTo("sub.login.bitwarden.com:1337"));
        Assert.That(coreHelpers.GetHost("https://localhost:8080"), Is.EqualTo("localhost:8080"));
        Assert.That(coreHelpers.GetHost("localhost"), Is.EqualTo(null));
        Assert.That(coreHelpers.GetHost("bitwarden"), Is.EqualTo(null));
        Assert.That(coreHelpers.GetHost("127.0.0.1"), Is.EqualTo("127.0.0.1"));
    }

    /// <summary>
    /// Returns the host (and not port) of the given uri.
    /// Does not support plain hostnames without a protocol.
    ///
    /// Input => Output examples:
    /// <para>https://bitwarden.com => bitwarden.com</para>
    /// <para>https://login.bitwarden.com:1337 => login.bitwarden.com</para>
    /// <para>https://sub.login.bitwarden.com:1337 => sub.login.bitwarden.com</para>
    /// <para>https://localhost:8080 => localhost</para>
    /// <para>localhost => null</para>
    /// <para>bitwarden => null</para>
    /// <para>127.0.0.1 => 127.0.0.1</para>
    /// </summary>
    [Test]
    public async Task Test_GetHostname()
    {
        var coreHelpers = await Create();

        Assert.That(coreHelpers.GetHostname("https://bitwarden.com"), Is.EqualTo("bitwarden.com"));
        Assert.That(coreHelpers.GetHostname("https://login.bitwarden.com:1337"), Is.EqualTo("login.bitwarden.com"));
        Assert.That(coreHelpers.GetHostname("https://sub.login.bitwarden.com:1337"), Is.EqualTo("sub.login.bitwarden.com"));
        Assert.That(coreHelpers.GetHostname("https://localhost:8080"), Is.EqualTo("localhost"));
        Assert.That(coreHelpers.GetHostname("localhost"), Is.EqualTo(null));
        Assert.That(coreHelpers.GetHostname("bitwarden"), Is.EqualTo(null));
        Assert.That(coreHelpers.GetHostname("127.0.0.1"), Is.EqualTo("127.0.0.1"));

        Assert.That(CoreHelpersOld.GetHostname("google.com"), Is.EqualTo("google.com"));
    }


    /// <summary>
    /// No test data specified
    /// </summary>
    [Test]
    public async Task Test_GetUri()
    {
        var coreHelpers = await Create();

        Assert.That(coreHelpers.GetHttpUri("https://bitwarden.com")?.ToString(), Is.EqualTo("https://bitwarden.com/"));
        Assert.That(coreHelpers.GetHttpUri("https://login.bitwarden.com:1337")?.ToString(), Is.EqualTo("https://login.bitwarden.com:1337/"));
        Assert.That(coreHelpers.GetHttpUri("https://sub.login.bitwarden.com:1337")?.ToString(), Is.EqualTo("https://sub.login.bitwarden.com:1337/"));
        Assert.That(coreHelpers.GetHttpUri("https://localhost:8080")?.ToString(), Is.EqualTo("https://localhost:8080/"));
        Assert.That(coreHelpers.GetHttpUri("localhost"), Is.EqualTo(null));
        Assert.That(coreHelpers.GetHttpUri("bitwarden"), Is.EqualTo(null));
        Assert.That(coreHelpers.GetHttpUri("127.0.0.1")?.ToString(), Is.EqualTo("http://127.0.0.1/"));
        Assert.That(coreHelpers.GetHttpUri("."), Is.EqualTo(null));
    }

    public async Task<CoreHelpers> Create()
    {
        var ruleProvider = new LocalFileRuleProvider("path_to/Resources/public_suffix_list.dat");
        await ruleProvider.BuildAsync();

        return new CoreHelpers(new DomainParser(ruleProvider));
    }
}

I used this Unit test class to check if my changes work as intended

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.