search

tilfinltd / aws-extend-switch-roles

Extend your AWS IAM switching roles by Chrome extension, Firefox add-on, or Edge add-on
https://chromewebstore.google.com/detail/aws-extend-switch-roles/jpmkfafbacpgapdghgdpembnojdlgkdl?utm_source=github
MIT License
1.32k stars 141 forks source link

Local storage for external API #147

Closed reegnz closed 3 years ago

reegnz commented 4 years ago

Fixes #146

codecov[bot] commented 4 years ago

Codecov Report

Merging #147 (af7ca6c) into master (3019afd) will increase coverage by 46.72%. The diff coverage is 26.92%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #147       +/-   ##
===========================================
+ Coverage   36.30%   83.02%   +46.72%     
===========================================
  Files           7       10        +3     
  Lines         146      324      +178     
===========================================
+ Hits           53      269      +216     
+ Misses         93       55       -38
Impacted Files Coverage Δ
src/lib/save_aws_config.js 0.00% <0.00%> (ø)
src/lib/content.js 95.10% <100.00%> (+87.10%) :arrow_up:
src/sanitizer.js 58.82% <0.00%> (ø)
src/lib/common.js 93.33% <0.00%> (ø)
src/lib/auto_assume_last_role.js 100.00% <0.00%> (+92.00%) :arrow_up:
src/lib/profile_set.js 100.00% <0.00%> (+100.00%) :arrow_up:

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 3019afd...922aef0. Read the comment docs.

reegnz commented 4 years ago

This also fixes #145 and #123 with a caveat, the large configs won't be synced anymore. Still better than not being able to use large configs at all IMHO.

reegnz commented 4 years ago

@tilfin I split out the saveAwsConfig function into a separate file because the same logic was used for both updating the config on the options page and through the extension API.

TryTryAgain commented 3 years ago

@reegnz have you been able to make this work with the latest updates that moved the accounts list to the extension popup?

Edit: I now have it working with pretty much the same/original PR you provided... Just needed to modify popup.js in a similar way you'd modified lib/content.js

cmug81 commented 3 years ago

Any idea when this will get merged? Pretty much a showstopper for us with a lot of profiles

reegnz commented 3 years ago

@TryTryAgain nope, the extension is still only using chrome.sync and never chrome.local.

https://github.com/tilfin/aws-extend-switch-roles/pull/147/files#diff-becaa957c81c7257d00f1688ea23241ea89eab618f3456abafc4e63e56c5a74bR29

The change I expect is using local storage when configs are coming from an external extension API.

reegnz commented 3 years ago

I did a rebase on the current master, and adjusted the code to work with the new popup. For some reason it's still broken though, so I'll attempt to fix.

reegnz commented 3 years ago

Okay now the changes work on HEAD, also removed the mandatory local storage in case of Extension API used.

reegnz commented 3 years ago

@tilfin please have a look at this PR and give feedback. This feature is strictly additive in nature, it's not breaking existing functionality.

andymac4182 commented 3 years ago

@tilfin Any chance of this being merged? It solves some big problems for us.

tilfin commented 3 years ago

I am prepared to deal with this issue.

andymac4182 commented 3 years ago

Thanks @tilfin Happy to help however we can.

tilfin commented 3 years ago

This PR feature wil be implemented on #193 and #194.