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

[spaceauth] add --copy_to_clipboard option to spaceauth #18538

Merged
merged 16 commits into from Apr 20, 2021
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 3 additions & 6 deletions fastlane/lib/fastlane/actions/clipboard.rb
Expand Up @@ -7,10 +7,7 @@ def self.run(params)
truncated_value = value[0..800].gsub(/\s\w+\s*$/, '...')
UI.message("Storing '#{truncated_value}' in the clipboard 🎨")

if FastlaneCore::Helper.mac?
require 'open3'
Open3.popen3('pbcopy') { |input, _, _| input << value }
end
FastlaneCore::Clipboard.copy(content: value)
end

#####################################################
Expand All @@ -30,11 +27,11 @@ def self.available_options
end

def self.authors
["KrauseFx", "joshdholtz"]
["KrauseFx", "joshdholtz", "rogerluan"]
end

def self.is_supported?(platform)
true
FastlaneCore::Clipboard.is_supported?
end

def self.example_code
Expand Down
43 changes: 22 additions & 21 deletions fastlane_core/lib/fastlane_core.rb
Expand Up @@ -3,39 +3,40 @@
require_relative 'fastlane_core/core_ext/string'
require_relative 'fastlane_core/core_ext/shellwords'

require_relative 'fastlane_core/analytics/action_completion_context'
rogerluan marked this conversation as resolved.
Show resolved Hide resolved
require_relative 'fastlane_core/analytics/action_launch_context'
require_relative 'fastlane_core/analytics/analytics_event_builder'
require_relative 'fastlane_core/analytics/analytics_ingester_client'
require_relative 'fastlane_core/analytics/analytics_session'
require_relative 'fastlane_core/build_watcher'
require_relative 'fastlane_core/cert_checker'
require_relative 'fastlane_core/clipboard'
require_relative 'fastlane_core/command_executor'
require_relative 'fastlane_core/configuration/configuration'
require_relative 'fastlane_core/device_manager'
require_relative 'fastlane_core/env'
require_relative 'fastlane_core/fastlane_folder'
require_relative 'fastlane_core/fastlane_pty'
require_relative 'fastlane_core/feature/feature'
require_relative 'fastlane_core/features'
require_relative 'fastlane_core/helper'
require_relative 'fastlane_core/configuration/configuration'
require_relative 'fastlane_core/update_checker/update_checker'
require_relative 'fastlane_core/languages'
require_relative 'fastlane_core/cert_checker'
require_relative 'fastlane_core/ipa_file_analyser'
require_relative 'fastlane_core/ipa_upload_package_builder'
require_relative 'fastlane_core/itunes_transporter'
require_relative 'fastlane_core/provisioning_profile'
require_relative 'fastlane_core/keychain_importer'
require_relative 'fastlane_core/languages'
require_relative 'fastlane_core/pkg_file_analyser'
require_relative 'fastlane_core/pkg_upload_package_builder'
require_relative 'fastlane_core/command_executor'
require_relative 'fastlane_core/ipa_upload_package_builder'
require_relative 'fastlane_core/print_table'
require_relative 'fastlane_core/project'
require_relative 'fastlane_core/device_manager'
require_relative 'fastlane_core/ui/ui'
require_relative 'fastlane_core/fastlane_folder'
require_relative 'fastlane_core/keychain_importer'
require_relative 'fastlane_core/provisioning_profile'
require_relative 'fastlane_core/queue_worker'
require_relative 'fastlane_core/swag'
require_relative 'fastlane_core/build_watcher'
require_relative 'fastlane_core/ui/errors'
require_relative 'fastlane_core/test_parser'
require_relative 'fastlane_core/analytics/action_completion_context'
require_relative 'fastlane_core/analytics/action_launch_context'
require_relative 'fastlane_core/analytics/analytics_event_builder'
require_relative 'fastlane_core/analytics/analytics_ingester_client'
require_relative 'fastlane_core/analytics/analytics_session'
require_relative 'fastlane_core/tag_version'
require_relative 'fastlane_core/fastlane_pty'
require_relative 'fastlane_core/queue_worker'
require_relative 'fastlane_core/test_parser'
require_relative 'fastlane_core/ui/errors'
require_relative 'fastlane_core/ui/ui'
require_relative 'fastlane_core/update_checker/update_checker'
Copy link
Contributor

Choose a reason for hiding this comment

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

I like reorganising dependencies alphabetically, but it may have some unintended side-effects in Ruby.
This PR works on my machine, but let's wait for CircleCI's unit tests to complete before merging.

Copy link
Member Author

@rogerluan rogerluan Apr 12, 2021

Choose a reason for hiding this comment

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

As long as the tests pass, is this safe? I have no clue 😮 what are/were the risks? maybe cc @ainame 🙏 😁

Copy link
Contributor

Choose a reason for hiding this comment

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

CircleCI passed, so it should be safe ✅

As you saw in fastlane_core.rb, some requires have to be ordered before:

# Ruby monkey-patches - should be before almost all else
require_relative 'fastlane_core/core_ext/string'
require_relative 'fastlane_core/core_ext/shellwords'

And some after:

require 'commander'

# after commander import
require_relative 'fastlane_core/ui/fastlane_runner' # monkey patch

They are usually extensions of Ruby classes, without those we'll probably get NoMethodErrors in CircleCI.

Copy link
Contributor

@ainame ainame Apr 13, 2021

Choose a reason for hiding this comment

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

As long as test cases cover those files, it would be fine🙂 Even in the monkey patch scenario mentioned, it doesn't matter as long as Ruby doesn't evaluate such patched methods directly.

For example, let's say we need to load two files in fastlane_core.rb and file A is like below, it is safe. The order doesn't matter. monkey_patch_method_return_true will be evaluated after everything loaded.

# fastlane_core.rb
require 'file A'
require 'monkey_patch'
# file A' 
class A 
  def hello
    puts("hello") if "some_string".monkey_patch_method_return_true
  end
end

However, file A' causes an error because we need to require file 'moneky_patch' that implements monkey_patch_method_return_true to String before file A is loaded.

# file A
if "some_string".monkey_patch_method_return_true
  class A 
    def hello; end
  end
end

commander scenario surely depends on the gem to define the classes first and then monkey patching follows.

You can test whether that file loading works by ruby -Ifastlane_core/lib fasltane_core/lib/fastlane_core.rb. Even so, there is a small chance order updates create logic-level discrepancy but unit tests should cover that 🤞


It's off-topic but fastlane's unit testing runs test cases exactly the same order every time right now so combining existing require order and unit test cases order may create a happy code path that still leaves some edge-cases technically😬

I've been trying to improve that in a PR...
#18278

Copy link
Member

Choose a reason for hiding this comment

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

This slightly terrifies me but I do prefer it this way 😬 I don’t think this should causes any issues but still a bit spooky 🤷‍♂️


# Third Party code
require 'colored'
Expand Down
20 changes: 20 additions & 0 deletions fastlane_core/lib/fastlane_core/clipboard.rb
@@ -0,0 +1,20 @@
require 'fastlane_core'
require 'open3'

module FastlaneCore
class Clipboard
def self.copy(content: nil)
return UI.crash!("'pbcopy' or 'pbpaste' command not found.") unless is_supported?
Open3.popen3('pbcopy') { |input, _, _| input << content }
end

def self.paste
return UI.crash!("'pbcopy' or 'pbpaste' command not found.") unless is_supported?
return `pbpaste`
end

def self.is_supported?
return `which pbcopy`.length > 0 && `which pbpaste`.length > 0
end
end
end
27 changes: 27 additions & 0 deletions fastlane_core/spec/clipboard_spec.rb
@@ -0,0 +1,27 @@
describe FastlaneCore do
describe FastlaneCore::Clipboard do
describe '#copy and paste' do
before(:each) do
@test_message = "_fastlane_ is awesome"
end

it 'should work on supported environments', if: FastlaneCore::Clipboard.is_supported? do
# Save clipboard
clipboard = FastlaneCore::Clipboard.paste

# Test copy and paste
FastlaneCore::Clipboard.copy(content: @test_message)
expect(FastlaneCore::Clipboard.paste).to eq(@test_message)

# Restore clipboard
FastlaneCore::Clipboard.copy(content: clipboard)
expect(FastlaneCore::Clipboard.paste).to eq(clipboard)
end

it 'should throw on non-supported environment', if: !FastlaneCore::Clipboard.is_supported? do
expect { FastlaneCore::Clipboard.copy(content: @test_message) }.to raise_error("'pbcopy' or 'pbpaste' command not found.")
expect { FastlaneCore::Clipboard.paste }.to raise_error("'pbcopy' or 'pbpaste' command not found.")
end
end
end
end
14 changes: 2 additions & 12 deletions spaceship/README.md
Expand Up @@ -111,21 +111,11 @@ When your Apple account has 2 factor verification enabled, you'll automatically

#### Web sessions

To generate a web session for your CI machine, use

```sh
fastlane spaceauth -u user@example.org
```

This will authenticate you and provide a string that can be transferred to your CI system. Copy everything from `---\n` to your CI server and provide it as environment variable named `FASTLANE_SESSION`. For example:

```
export FASTLANE_SESSION='---\n- !ruby/object:HTTP::Cookie\n name: DES5c148586dfd451e55afbaaa5f62418f91\n value: HSARMTKNSRVTWFla1+yO4gVPowH17VaaaxPFnUdMUegQZxqy1Ie1c2v6bM1vSOzIbuOmrl/FNenlScsd/NbF7/Lw4cpnL15jsyg0TOJwP32tC/NguPiyOaaaU+jrj4tf4uKdIywVaaaFSRVT\n domain: idmsa.apple.com\n for_domain: true\n path: "/"\n secure: true\n httponly: true\n expires: 2016-04-27 23:55:56.000000000 Z\n max_age: \n created_at: 2016-03-28 16:55:57.032086000 -07:00\n accessed_at: 2016-03-28 19:11:17.828141000 -07:00\n'
```
See [Continuous Integration > Authenticating with Apple services > Method 2: Two-step or two-factor authentication > Storing a manually verified session using spaceauth](https://docs.fastlane.tools/best-practices/continuous-integration/#storing-a-manually-verified-session-using-spaceauth)

#### Transporter

See [Continuous Integration > Authentication with Apple services > Application specific passwords](https://docs.fastlane.tools/best-practices/continuous-integration/#application-specific-passwords)
See [Continuous Integration > Authenticating with Apple services > Method 3: Application-specific passwords](https://docs.fastlane.tools/best-practices/continuous-integration/#method-3-application-specific-passwords)
rogerluan marked this conversation as resolved.
Show resolved Hide resolved

## _spaceship_ in use

Expand Down
3 changes: 2 additions & 1 deletion spaceship/lib/spaceship/commands_generator.rb
Expand Up @@ -39,9 +39,10 @@ def run
command :spaceauth do |c|
c.syntax = 'fastlane spaceship spaceauth'
c.description = 'Authentication helper for spaceship/fastlane to work with Apple 2-Step/2FA'
c.option('--copy_to_clipboard', 'Whether the session string should be copied to clipboard. For more info see https://docs.fastlane.tools/best-practices/continuous-integration/#storing-a-manually-verified-session-using-spaceauth`')

c.action do |args, options|
Spaceship::SpaceauthRunner.new(username: options.user).run
Spaceship::SpaceauthRunner.new(username: options.user, copy_to_clipboard: options.copy_to_clipboard).run
end
end

Expand Down
28 changes: 19 additions & 9 deletions spaceship/lib/spaceship/spaceauth_runner.rb
@@ -1,15 +1,17 @@
require 'colored'
require 'credentials_manager/appfile_config'
require 'yaml'
require 'fastlane_core'

require_relative 'tunes/tunes_client'

module Spaceship
class SpaceauthRunner
def initialize(username: nil)
def initialize(username: nil, copy_to_clipboard: nil)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def initialize(username: nil, copy_to_clipboard: nil)
def initialize(username: nil, copy_to_clipboard: false)

Copy link
Contributor

Choose a reason for hiding this comment

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

That would change the default behaviour...
See #18538 (comment)

Copy link
Member Author

@rogerluan rogerluan Apr 19, 2021

Choose a reason for hiding this comment

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

☝️ Yeah, here the absence of the param behaves differently than an explicit false value, hence why the check up there is == false instead of a 'falsey' check

@username = username
@username ||= CredentialsManager::AppfileConfig.try_fetch_value(:apple_id)
@username ||= ask("Username: ")
@copy_to_clipboard = copy_to_clipboard
end

def run
Expand All @@ -22,7 +24,7 @@ def run
puts("Could not login to App Store Connect".red)
puts("Please check your credentials and try again.".yellow)
puts("This could be an issue with App Store Connect,".yellow)
puts("Please try unsetting the FASTLANE_SESSION environment variable".yellow)
puts("Please try unsetting the FASTLANE_SESSION environment variable by calling 'unset FASTLANE_SESSION'".yellow)
puts("(if it is set) and re-run `fastlane spaceauth`".yellow)
puts("")
puts("Exception type: #{ex.class}")
Expand All @@ -49,22 +51,30 @@ def run
cookie.name.start_with?("myacinfo") || cookie.name == "dqsid" || cookie.name.start_with?("DES")
end

yaml = cookies.to_yaml.gsub("\n", "\\n")
@yaml = cookies.to_yaml.gsub("\n", "\\n")

puts("---")
puts("")
puts("Pass the following via the FASTLANE_SESSION environment variable:")
puts(yaml.cyan.underline)
puts(@yaml.cyan.underline)
puts("")
puts("")
puts("Example:")
puts("export FASTLANE_SESSION='#{yaml}'".cyan.underline)
puts("export FASTLANE_SESSION='#{@yaml}'".cyan.underline)

if mac? && Spaceship::Client::UserInterface.interactive? && agree("🙄 Should fastlane copy the cookie into your clipboard, so you can easily paste it? (y/n)", true)
require 'open3'
Open3.popen3('pbcopy') { |input, _, _| input << yaml }
puts("Successfully copied text into your clipboard 🎨".green)
if @copy_to_clipboard == false
puts("Skipped asking to copy the session string into your clipboard ⏭️".green)
elsif @copy_to_clipboard || (mac? && Spaceship::Client::UserInterface.interactive? && agree("🙄 Should fastlane copy the cookie into your clipboard, so you can easily paste it? (y/n)", true))
FastlaneCore::Clipboard.copy(content: @yaml)
puts("Successfully copied the session string into your clipboard 🎨".green)
end

return self
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for that, it simplified my logic to:

fastlane_session = Spaceship::SpaceauthRunner.new().run.instance_variable_get(:@yaml)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I just noticed the session_string method.
So the proper way is:

fastlane_session = Spaceship::SpaceauthRunner.new().run.session_string

Copy link
Member

Choose a reason for hiding this comment

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

Can this just return @yaml? 😇 This is already running on an instance so I don’t think returning self is as useful as returning the actual value? 🤔

This also gets rid of the need for the session_string addition below 🤷‍♂️

cc: @rogerluan @return-main

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be okay with that, but by returning self we can future-proof the lane in case we create new methods.

Copy link
Member Author

@rogerluan rogerluan Apr 19, 2021

Choose a reason for hiding this comment

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

I share the same thoughts as @return-main ☝️
Not sure how/if this class plans to be changed much in the future, but it's a possibility. It would make it possible to create new methods similar to how session_string works, without worrying about retrocompatibility.

What do you think @joshdholtz ?

Copy link
Member

Choose a reason for hiding this comment

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

I’m fine with that 😇 We should be good to go then

end

def session_string
FastlaneCore::UI.user_error!("`#{__method__}` method called before calling `run` in `SpaceauthRunner`") unless @yaml
@yaml
end

def mac?
Expand Down
36 changes: 36 additions & 0 deletions spaceship/spec/spaceauth_spec.rb
@@ -1,4 +1,5 @@
require_relative 'tunes/tunes_stubbing'
require 'fastlane_core/clipboard'

describe Spaceship::SpaceauthRunner do
let(:user_cookie) { TunesStubbing.itc_read_fixture_file('spaceauth_cookie.yml') }
Expand All @@ -13,4 +14,39 @@
Spaceship::SpaceauthRunner.new.run
end.to output(/export FASTLANE_SESSION=.*name: DES.*name: myacinfo.*name: dqsid.*/).to_stdout
end

describe 'copy_to_clipboard option', if: FastlaneCore::Clipboard.is_supported? do
before :each do
# Save clipboard
@clipboard = FastlaneCore::Clipboard.paste
end

after :each do
# Restore clipboard
FastlaneCore::Clipboard.copy(content: @clipboard)
end

it 'when true, it should copy the session to clipboard' do
Spaceship::SpaceauthRunner.new(copy_to_clipboard: true).run
expect(FastlaneCore::Clipboard.paste).to match(%r{.*domain: idmsa.apple.com.*path: \"\/appleauth\/auth\/\".*})
end

it 'when false, it should not copy the session to clipboard' do
Spaceship::SpaceauthRunner.new(copy_to_clipboard: false).run
expect(FastlaneCore::Clipboard.paste).to eq(@clipboard)
end
end

describe '#session_string' do
it 'should return the session when called after run' do
expect(Spaceship::SpaceauthRunner.new.run.session_string).to match(%r{.*domain: idmsa.apple.com.*path: \"\/appleauth\/auth\/\".*})
end

it 'should throw when called before run' do
expect(FastlaneCore::UI).to receive(:user_error!).with(/method called before calling `run` in `SpaceauthRunner`/).and_raise("boom")
expect do
Spaceship::SpaceauthRunner.new.session_string
end.to raise_error("boom")
end
end
end