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

add autofill command #831

Merged
merged 2 commits into from
Apr 25, 2022
Merged

add autofill command #831

merged 2 commits into from
Apr 25, 2022

Conversation

Sneezry
Copy link
Member

@Sneezry Sneezry commented Jan 6, 2022

@codecov
Copy link

codecov bot commented Jan 6, 2022

Codecov Report

Merging #831 (31d6d9d) into dev (05dbda0) will decrease coverage by 0.20%.
The diff coverage is 13.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #831      +/-   ##
==========================================
- Coverage   18.34%   18.13%   -0.21%     
==========================================
  Files          17       18       +1     
  Lines        1543     1544       +1     
  Branches      337      338       +1     
==========================================
- Hits          283      280       -3     
- Misses       1226     1230       +4     
  Partials       34       34              
Impacted Files Coverage Δ
src/utils.ts 11.86% <11.86%> (ø)
src/store/Accounts.ts 7.92% <50.00%> (-1.90%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05dbda0...31d6d9d. Read the comment docs.

@mymindstorm
Copy link
Member

So, the reason we didn't implement this before was because we don't have secure website matching like the password managers do. We can't be sure that a website will not intentionally trick our auto fill matching and pretend to be a legitimate website. Do you think that is a big enough issue to not implement this feature?

@Sneezry
Copy link
Member Author

Sneezry commented Jan 16, 2022

So, the reason we didn't implement this before was because we don't have secure website matching like the password managers do. We can't be sure that a website will not intentionally trick our auto fill matching and pretend to be a legitimate website. Do you think that is a big enough issue to not implement this feature?

The current design is if there is only one matched account, autofill command will fill the code, or nothing happens. I don't think it's a big problem because in most cases, there's only one matched account. Smart filter works well, and secure website matching is not so important. FYI, this PR is for this feedback #829

@philippthiele
Copy link

philippthiele commented Feb 10, 2022

I am a little confused, my authenticator extension already has such an option, I enabled it to use autofill, but it doesn't do anything. Now I see here it is not really merged yet. What does the current autofill option do?

EDIT:
Oh I see now, I had to restart chrome and now when I click the code it autofills. Sorry for the too quick question. But I will be very interested to see this feature here merged, I think it is a good addition, since I already thought this is what it is doing with the current autofill.

const entries = await EntryStorage.get();
const matchedEntries = getMatchedEntries(siteName, entries);

if (matchedEntries && matchedEntries.length === 1) {
Copy link
Member

Choose a reason for hiding this comment

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

matchedEntries.length <= 1

Copy link
Member Author

Choose a reason for hiding this comment

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

if matchedEntries.length == 0, const entry = matchedEntries[0] will be undefined. We should auto fill code when there is only one matched entry

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, incorrect logic on my end. I meant matchedEntries.length >= 1. It won't past a code if there is more than one match.

Copy link
Member Author

Choose a reason for hiding this comment

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

If there is more than one matched entry I think we should always ask the user to choose the correct one, but not the first one. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I agree, that would be the best solution.

This comment was marked as duplicate.

src/background.ts Outdated Show resolved Hide resolved
@Haiyangha

This comment has been minimized.

1 similar comment
@Haiyangha

This comment has been minimized.

@Jeunesse16

This comment was marked as duplicate.

@Sneezry Sneezry merged commit 2f897c9 into dev Apr 25, 2022
@Sneezry Sneezry deleted the autofill-command branch April 25, 2022 02:59
Sneezry added a commit that referenced this pull request May 27, 2024
* add compact theme

* Add new strings

This commit was automatically made by run 1641549669

* add icon for shortcut (#820)

* strict ts

* managed storage not available in safari.

* use blob url to replace data url (#817)

* Add flat theme.

* add permissions management (#827)

* add permissions management

* hide required permission by default

* update permission description

* update strings

Co-authored-by: Brendan Early <mymindstorm@evermiss.net>

* Add new strings

This commit was automatically made by run 1704482678

* Bump pathval from 1.1.0 to 1.1.1 (#852)

Bumps [pathval](https://github.com/chaijs/pathval) from 1.1.0 to 1.1.1.
- [Release notes](https://github.com/chaijs/pathval/releases)
- [Changelog](https://github.com/chaijs/pathval/blob/master/CHANGELOG.md)
- [Commits](chaijs/pathval@v1.1.0...v1.1.1)

---
updated-dependencies:
- dependency-name: pathval
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

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

* Bump node-fetch from 2.6.1 to 2.6.7 (#853)

Bumps [node-fetch](https://github.com/node-fetch/node-fetch) from 2.6.1 to 2.6.7.
- [Release notes](https://github.com/node-fetch/node-fetch/releases)
- [Commits](node-fetch/node-fetch@v2.6.1...v2.6.7)

---
updated-dependencies:
- dependency-name: node-fetch
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

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

* Bump urijs from 1.19.7 to 1.19.10 (#873)

Bumps [urijs](https://github.com/medialize/URI.js) from 1.19.7 to 1.19.10.
- [Release notes](https://github.com/medialize/URI.js/releases)
- [Changelog](https://github.com/medialize/URI.js/blob/gh-pages/CHANGELOG.md)
- [Commits](medialize/URI.js@v1.19.7...v1.19.10)

---
updated-dependencies:
- dependency-name: urijs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

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

* Bump minimist from 1.2.5 to 1.2.6 (#884)

Bumps [minimist](https://github.com/substack/minimist) from 1.2.5 to 1.2.6.
- [Release notes](https://github.com/substack/minimist/releases)
- [Commits](https://github.com/substack/minimist/compare/1.2.5...1.2.6)

---
updated-dependencies:
- dependency-name: minimist
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

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

* Bump ansi-regex from 3.0.0 to 3.0.1 (#885)

Bumps [ansi-regex](https://github.com/chalk/ansi-regex) from 3.0.0 to 3.0.1.
- [Release notes](https://github.com/chalk/ansi-regex/releases)
- [Commits](chalk/ansi-regex@v3.0.0...v3.0.1)

---
updated-dependencies:
- dependency-name: ansi-regex
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

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

* Bump urijs from 1.19.10 to 1.19.11 (#886)

Bumps [urijs](https://github.com/medialize/URI.js) from 1.19.10 to 1.19.11.
- [Release notes](https://github.com/medialize/URI.js/releases)
- [Changelog](https://github.com/medialize/URI.js/blob/gh-pages/CHANGELOG.md)
- [Commits](medialize/URI.js@v1.19.10...v1.19.11)

---
updated-dependencies:
- dependency-name: urijs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

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

* add autofill command (#831)

* add autofill command

* address comment

* Add 'wasm-unsafe-eval' to custom CSP (fix #906)

It seems that some dependencies (at least argon2-browser) require allowing WebAssembly at the CSP level.
Recent Firefox allows that by default, but having your own CSP means this extension won't be able to benefit from this automatically.

* revert unnecessary change

* Update readme

* Browser componnet

* isFirefox

* isEdge.

* 💄

* TypeScript does not like type annotation for catch.

* 💄

* fix build

* update theme for safari

* fix local download

* update to latest

* minimal change

* 💄

* fix redundant theme option

* Update translations from crowdin

* Update readme for safari.

* Use `&&`

* Add new strings

This commit was automatically made by run 2680600562

* Bump terser from 4.8.0 to 4.8.1 (#929)

Bumps [terser](https://github.com/terser/terser) from 4.8.0 to 4.8.1.
- [Release notes](https://github.com/terser/terser/releases)
- [Changelog](https://github.com/terser/terser/blob/master/CHANGELOG.md)
- [Commits](https://github.com/terser/terser/commits)

---
updated-dependencies:
- dependency-name: terser
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

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

* Bump loader-utils from 1.4.0 to 1.4.1 (#970)

Bumps [loader-utils](https://github.com/webpack/loader-utils) from 1.4.0 to 1.4.1.
- [Release notes](https://github.com/webpack/loader-utils/releases)
- [Changelog](https://github.com/webpack/loader-utils/blob/v1.4.1/CHANGELOG.md)
- [Commits](webpack/loader-utils@v1.4.0...v1.4.1)

---
updated-dependencies:
- dependency-name: loader-utils
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump qs from 6.9.4 to 6.11.0 (#985)

Bumps [qs](https://github.com/ljharb/qs) from 6.9.4 to 6.11.0.
- [Release notes](https://github.com/ljharb/qs/releases)
- [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md)
- [Commits](ljharb/qs@v6.9.4...v6.11.0)

---
updated-dependencies:
- dependency-name: qs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump json5 from 1.0.1 to 1.0.2 (#993)

Bumps [json5](https://github.com/json5/json5) from 1.0.1 to 1.0.2.
- [Release notes](https://github.com/json5/json5/releases)
- [Changelog](https://github.com/json5/json5/blob/main/CHANGELOG.md)
- [Commits](json5/json5@v1.0.1...v1.0.2)

---
updated-dependencies:
- dependency-name: json5
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump cookiejar from 2.1.2 to 2.1.4 (#1006)

Bumps [cookiejar](https://github.com/bmeck/node-cookiejar) from 2.1.2 to 2.1.4.
- [Release notes](https://github.com/bmeck/node-cookiejar/releases)
- [Commits](https://github.com/bmeck/node-cookiejar/commits)

---
updated-dependencies:
- dependency-name: cookiejar
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix disable backup policy not working issue (#1050)

* 6.3.5

* typo

* fix typo (#1144)

* add import QR images/OTP URLs buttons to add account page

* Add new strings

This commit was automatically made by run 8076854721

* Migrate to MV3 (Chrome, Firefox, Edge) (#1009)

* Migrate to MV3

* fix entry type

* fix default storage area

* fix mv3 migration errors

* further CI fixes

* Fix disable backup policy not working issue (#1050)

* 6.3.5

* typo

* Fix bad practice with argon2-browser

* Remove 'offline_enabled' from manifest

* update test runner and coverage for mv3

* don't use dev config by default

* fix typo (#1144)

* add import QR images/OTP URLs buttons to add account page

* Migrate to MV3

* fix entry type

* fix default storage area

* fix mv3 migration errors

* further CI fixes

* Fix bad practice with argon2-browser

* Remove 'offline_enabled' from manifest

* update test runner and coverage for mv3

* don't use dev config by default

* dump version

* update manifest

* fix ff csp

* remove artifact

* rename test files

* move syncTimeWithGoogle out of popup.ts

this prevents a dependency issue in the tests

* fix the tests, remove code coverage

mv3 makes code cov w/ istanbul virtually impossible due to restrictions on unsafe-eval

* remove testing code

* refactor user settings (#1191)

* remove out-of-date eslint comments

* fix user settings migration issue

* fix user setting migration issue

* fix edge errors

* fix edge issues

* update firefox permissions

* remove all_urls permission since Firefox has supported activeTab

* fix firefox crash due to functions getting added to usersettings object

---------

Co-authored-by: Brendan Early <mymindstorm@evermiss.net>
Co-authored-by: Zhe Li <Sneezry@users.noreply.github.com>
Co-authored-by: spaette <111918424+spaette@users.noreply.github.com>
Co-authored-by: vuittont60 <81072379+vuittont60@users.noreply.github.com>

* Add new strings

This commit was automatically made by run 9249787733

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Zhe Li <Sneezry@users.noreply.github.com>
Co-authored-by: rebornix <penn.lv@gmail.com>
Co-authored-by: Brendan Early <mymindstorm@evermiss.net>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Frederik Braun <fbraun+gh@mozilla.com>
Co-authored-by: Peng Lyu <penlv@microsoft.com>
Co-authored-by: spaette <111918424+spaette@users.noreply.github.com>
Co-authored-by: vuittont60 <81072379+vuittont60@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

5 participants