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

[Feathure] Hide pin input support #6

Closed
Halimao opened this issue Nov 15, 2022 · 7 comments · Fixed by #7
Closed

[Feathure] Hide pin input support #6

Halimao opened this issue Nov 15, 2022 · 7 comments · Fixed by #7

Comments

@Halimao
Copy link

Halimao commented Nov 15, 2022

--pin flag doesn't hide pin content now, in my opinion, this is not secure.

using --pin without leave pin content, return error message following:
execute failed: flag needs an argument: --pin

@Halimao
Copy link
Author

Halimao commented Nov 15, 2022

By the way, seems that v2 has remove "update pin" sub command?

@xwjdsh
Copy link
Contributor

xwjdsh commented Nov 15, 2022

You can add the pin to your Keystore file, and then you don't need to set a pin from the command line.

{
  "pin":"CHANGE_ME",
  "client_id": "..."
  ...
}

@Halimao
Copy link
Author

Halimao commented Nov 15, 2022

You can add the pin to your Keystore file, and then you don't need to set a pin from the command line.

{
  "pin":"CHANGE_ME",
  "client_id": "..."
  ...
}

But store pin in the file is not secure too... also, fox-one/mixin-sdk-go doestn't store pin in keystore

@xwjdsh
Copy link
Contributor

xwjdsh commented Nov 15, 2022

fox-one/mixin-sdk-go doestn't store pin in keystore

Here it is https://github.com/fox-one/mixin-cli/blob/master/cmdutil/store.go#L16

You make sense. If the pin is required but is not set on the command line and in the file, maybe we should let the user provide pin input instead of returning an error.

@yiplee What's your opinion? I'd like to send PR if you agree.

@lyricat
Copy link
Contributor

lyricat commented Nov 15, 2022

maybe we should let the user provide pin input instead of returning an error.

vote for that

@Halimao
Copy link
Author

Halimao commented Nov 15, 2022

By the way, seems that v2 has remove "update pin" sub command?

"update pin" command is also helpful ☺

@yiplee
Copy link
Collaborator

yiplee commented Nov 15, 2022

If the pin is required but is not set on the command line and in the file, maybe we should let the user provide pin input instead of returning an error.

sounds good to me 👍 @xwjdsh

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 a pull request may close this issue.

4 participants