Fix issue where CLI commands making more than one request to rubygems.org needing an OTP code would crash or ask for the code twice

[sonalkr132]

What was the end-user or developer problem that led to this PR?

Fixes failed commands with access update or fist invocation (no local API key):

$ gem yank somegem -v 0.0.1
Don't have an account yet? Create one at http://localhost:3000/sign_up
   Email:   some@rubygems.org
  Password:   
You have enabled multi-factor authentication. Please enter OTP code.
Code:   320292
Added yank_rubygem scope to the existing API key
You have enabled multifactor authentication but no OTP code provided.
Please fill it and retry.

What is your fix for the problem, implemented in this PR?

reuse otp passed by the user in api request retry

Make sure the following tasks are checked

• Describe the problem / feature
• Write tests for features and bug fixes
• Write code to solve the problem
• Make sure you follow the current code style and write meaningful commit messages without tags

[sonalkr132]

The first invocation has one more quirk, which exists in previous versions as well:

Don't have an account yet? Create one at http://localhost:3000/sign_up
   Email:   some@rubygems.org
Password:   

You have enabled multi-factor authentication. Please enter OTP code.
Code:   569043
Signed in.
Pushing gem to http://localhost:3000...
You have enabled multi-factor authentication. Please enter OTP code.
Code:   569043
Successfully registered gem: sometsomeee (3.10.50.15)

It asks for OTP twice because sign_in and real execution are separate invocations(1). Is it okay to use options[:otp] as an instance variable, which will be set in sign_in?

[deivid-rodriguez]

It asks for OTP twice because sign_in and real execution are separate invocations(1). Is it okay to use options[:otp] as an instance variable, which will be set in sign_in?

It seems ok to me!

Something I don't understand is how the scope is getting updated in the current version if it needs an otp code too?

[sonalkr132]

update scope was promoting for otp, through rubygems_api_request. This version has one fewer prompt (and request) for update scope case.

[deivid-rodriguez]

Aahhhh right, sorry, I get it now.

[deivid-rodriguez]

This looks good to me!

Another quirk I see (I haven't tried it but I think it happes from reading the code) is that if --opt is given on the command line, the given code will be used for sign_in, but then other requests will still ask for an otp code, right?

If we implement your instance variable suggestion, we're it's filled up lazily from either options[:otp] or asking the user, I think both cases would be fixed?

In any case, this PR keeps existing behaviour and fixes an issue, so if you want to merge this now and fix the other cases later, I'm good with it!

[deivid-rodriguez]

@sonalkr132 Not sure if you're still there, but if you give me the OK now, I'll sneak this into the 3.2.2 release.

[sonalkr132]

the given code will be used for sign_in, but then other requests will still ask for an otp code, right?

not really. for them the otp is passed from the command file itself (1). if mfa_unauthorized?(response) is basically fallback.

RUBYGEMS_HOST="https://staging.rubygems.org" gem push sometsomeee-3.10.50.15.gem  --otp 487877
Enter your https://staging.rubygems.org credentials.
Don't have an account yet? Create one at https://staging.rubygems.org/sign_up
   Email:   aditya.prakash132@gmail.com
Password:   

Signed in.
Pushing gem to https://staging.rubygems.org...
Successfully registered gem: sometsomeee (3.10.50.15)

[sonalkr132]

this assert fails with nil without b09e932

[deivid-rodriguez]

Looks good to me @sonalkr132!

[deivid-rodriguez]

I'd like to rename the PR to something more close to the user problem being fixed, maybe: "Fix issue where CLI commands making more than one request to rubygems.org needing an OTP code would crash or ask for the code twice"?

[deivid-rodriguez]

Anyways, merging.

Thanks for fixing this and improving the code all around!

[deivid-rodriguez]

To clarify why I'd like to rename the PR, our scripts use the PR title to generate the changelog, so the title of the PR is what the users will see when checking the changelog. So the idea would be that PR titles are end user focused. No big deal though!