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

Wipe login data #39

Closed
wants to merge 2 commits into from
Closed

Wipe login data #39

wants to merge 2 commits into from

Conversation

tijno
Copy link
Contributor

@tijno tijno commented Jun 9, 2021

This is the PR as per issue #34

It will clear login info from local storage for all logged in accounts after the user enters a confirmation sentence.

cc @HPaulson

@tijno tijno requested a review from a team as a code owner June 9, 2021 20:36
Copy link

@HPaulson HPaulson left a comment

Choose a reason for hiding this comment

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

After a quick read of the changes Everything LGTM -- Note though that I'm able to test this locally (as I broke my machine 😬), however I assume you've done so already based on the gif provided in #34.

This doesn't solve the major security concerns which Identity presents, but small additions like this definitely make things a bit easier for the end user. Thanks for the PR!

@tijno tijno mentioned this pull request Jun 11, 2021
@tijno
Copy link
Contributor Author

tijno commented Jun 14, 2021

anything else needed in this @maebeam @lazynina

bunch of people got hacked yesterday due to using extensions and having old accounts logged in.

not being able to clear local storage easily makes this worse when it happens.

@maebeam
Copy link
Contributor

maebeam commented Jun 14, 2021

@tijno I'd like to make this a separate component e.g. /logout

Which extensions were compromised?

@tijno
Copy link
Contributor Author

tijno commented Jun 14, 2021

ok will make it separate component.

not sure which ones - @HPaulson looked into that side of the hacked accounts. I could see about 20 likely affected.

@maebeam
Copy link
Contributor

maebeam commented Jun 14, 2021

thank you!

@iPaulPro
Copy link
Contributor

Has anyone provided any proof or identified specific extensions? Seemed like some opportunist FUD to me.

"Someone sent all my CLOUT to an anon address." Please donate. 😂

@0mniman
Copy link

0mniman commented Jun 18, 2021

Has anyone provided any proof or identified specific extensions? Seemed like some opportunist FUD to me.

"Someone sent all my CLOUT to an anon address." Please donate. 😂

I had so much loosing money then asking people in discord to help. Its laugh for you because you are not a from third world country. Laughing off people losses, what a shame. And mentioning as I asking for something that wasn't mine.

@maebeam
Copy link
Contributor

maebeam commented Jul 8, 2021

@tijno if you have some time I'd love to get this merged

@tijno
Copy link
Contributor Author

tijno commented Jul 8, 2021

ill do my best this weekend @maebeam

I'd like to make this a separate component e.g. /logout

time to learn some angular.

@akutch
Copy link

akutch commented Jul 9, 2021

I might have time to split this out into a separate component this afternoon. I'll post here before I do just in case you get to it first @tijno

akutch added a commit to akutch/identity that referenced this pull request Jul 9, 2021
@tijno
Copy link
Contributor Author

tijno commented Jul 9, 2021

earliest i will get to it is sunday @akutch

this would be so amazing 🤩

@akutch
Copy link

akutch commented Jul 9, 2021

@tijno ok I'm starting it today 🙂 I'll see how far I can get

Couple clarifying questions – 

  1. The work left is just a matter of splitting the 'clear' logic into a separate component (e.g. clear-account), right?
  2. Are we thinking this is a separate route as well or just part of log-in?

Draft PR here: #51

@tijno
Copy link
Contributor Author

tijno commented Jul 9, 2021

@akutch

yes separate component

and i think separate route too as "signup" for example from memory is a separate route too. So keep consistent with that.

@akutch akutch mentioned this pull request Jul 9, 2021
@akutch
Copy link

akutch commented Jul 10, 2021

The changes are ready for review here: #51

@tijno
Copy link
Contributor Author

tijno commented Jul 11, 2021

included in PR mentioned above by @akutch so closing this one.

@tijno tijno closed this Jul 11, 2021
@tijno tijno deleted the wipe-login-data branch July 11, 2021 17:16
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.

6 participants