-
Notifications
You must be signed in to change notification settings - Fork 111
fix(sec-bug): handle no-login mode extensively #2615
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
base: dev
Are you sure you want to change the base?
Changes from 8 commits
13a776f
f86da05
0287712
d94f603
e6088cb
bf596dc
3ea4849
f0824e6
a9af8e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -296,6 +296,8 @@ pub enum EnablePlatformCoinWithTokensError { | |
| #[display(fmt = "WalletConnect Error: {_0}")] | ||
| WalletConnectError(String), | ||
| PlatformCoinMismatch, | ||
| #[display(fmt = "Cannot enable coins in no-login mode.")] | ||
| CannotEnableInNoLoginMode, | ||
| } | ||
|
|
||
| impl From<CoinConfWithProtocolError> for EnablePlatformCoinWithTokensError { | ||
|
|
@@ -390,6 +392,7 @@ impl HttpStatusCode for EnablePlatformCoinWithTokensError { | |
| | EnablePlatformCoinWithTokensError::FailedSpawningBalanceEvents(_) | ||
| | EnablePlatformCoinWithTokensError::WalletConnectError(_) | ||
| | EnablePlatformCoinWithTokensError::PlatformCoinMismatch | ||
| | EnablePlatformCoinWithTokensError::CannotEnableInNoLoginMode | ||
| | EnablePlatformCoinWithTokensError::UnexpectedTokenProtocol { .. } => StatusCode::BAD_REQUEST, | ||
| EnablePlatformCoinWithTokensError::Transport(_) => StatusCode::BAD_GATEWAY, | ||
| } | ||
|
|
@@ -441,6 +444,10 @@ where | |
| Platform: PlatformCoinWithTokensActivationOps, | ||
| EnablePlatformCoinWithTokensError: From<Platform::ActivationError>, | ||
| { | ||
| if ctx.is_no_login_mode() { | ||
| return MmError::err(EnablePlatformCoinWithTokensError::CannotEnableInNoLoginMode); | ||
| } | ||
|
Comment on lines
+447
to
+449
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that you did add the check for some of the v2 activation methods, but I think that we need it in the utxo v2 activation methods as well.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this a main gateway for all the v2 activations?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Apparently it's not, what the hell..? They live under the same module but not implement the activation traits...
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any activation or private-key related logic that isn't explicitly handled will eventually be covered by this check so we don't need to add |
||
|
|
||
| if req.request.is_hw_policy() { | ||
| return MmError::err(EnablePlatformCoinWithTokensError::UnexpectedDeviceActivationPolicy); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need something similar in v2 activation methods?