-
Notifications
You must be signed in to change notification settings - Fork 46
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
chore: Migrate Two FA from packngo to equinix-sdk-go client #353
Conversation
@aayushrangwala please open an issue and create a patches/ PR in Metal Go for the missing TFA parameters (which we can piece together from the packngo implementation). We can rebase this PR against metal go with those changes. |
This is one of two final |
Yes, we do have an issue open with API team. Although seems it has not been prioritised yet |
…25) Conversion of [`metal-cli` 2-factor auth subcommands](equinix/metal-cli#353) from `packngo` to `equinix-sdk-go` is blocked because the Equinix Metal OpenAPI spec is missing many details for MFA setup and validation. This works around the API issue by patching the local SDK spec to fill in the missing MFA details. These patches were written based on the [`packngo` MFA implementation](https://github.com/packethost/packngo/blob/f8b3f36ba929a29d4f1d29480e0f47c7f85790e4/two_factor_auth.go).
|
cmdFunc func(*testing.T, *cobra.Command) | ||
}{ | ||
{ | ||
name: "receive sms", |
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.
Note: this is only testing the receive subcommand because I haven't recreated a successful scenario with the enable
and disable
commands. For the other 2fa subcommands, the same failure happens in both the packngo and equinix-sdk-go implementations:
HTTP/1.1 424 Failed Dependency
{"errors":["Invalid OTP"]}
Since this is a pre-existing issue and isn't caused or fixed by the SDK migration, I'm advocating to handle it in a separate issue & PR.
Is #59 also addressed by this PR? |
This PR does not address that issue but that issue appears to have already been addressed. |
Signed-off-by: Ayush Rangwala <ayush.rangwala@gmail.com>
Issue Task as part of migrating metal-cli from packngo to metal-go client, added the support of Two Fa to use metal-go
Fixes: #333
Discussion:
As of metal-go
0.22.2
there are 2 issues which needs api supportotp code
in the input forEnable and Disable 2FA
is not supported from metal-gootp
on two fa registeredapp
is also not supported