[match] import P12 codesigning certificate to match's repo, updated, fixed #9752

razum2um · 2017-07-14T05:38:43Z

Checklist

I've run bundle exec rspec from the root directory to see all new and existing tests pass
I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
I've read the Contribution Guidelines
I've updated the documentation if necessary.

Motivation and Context

Asked here #6900 and continued here #6972

Description

In comparison to #6972 it matches certificates by issuer and serial which according to rfc identify a unique certificate, I also addressed all the comments from previous version

googlebot · 2017-07-14T05:38:47Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

razum2um · 2017-07-14T05:40:23Z

I signed it!

googlebot · 2017-07-14T05:40:25Z

CLAs look good, thanks!

razum2um · 2017-07-14T05:49:26Z

@milch I addressed your comments on the prev PR please take a look :)

ByKeks · 2017-07-31T06:30:27Z

need this very much

endocrimes

Looking good, I have a few grammar/spelling tweaks and some feature questions.

endocrimes · 2017-08-01T13:21:18Z

match/README.md

@@ -327,7 +327,7 @@ Once you've decided which approach to take, all that's left to do is to set your

 ### Nuke

-If you never really cared about code signing and have a messy Apple Developer account with a lot of invalid, expired or Xcode managed profiles/certificates, you can use the `match nuke` command to revoke your certificates and provisioning profiles. Don't worry, apps that are already available in the App Store / TestFlight will still work. Builds distributed via Ad Hoc or Enterprise will be disabled after nuking your account, so you'll have to re-upload a new build. After clearing your account you'll start from a clean state, and you can run `match` to generate your certificates and profiles again.
+If you never really cared about code signing and have a messy Apple Developer account with a lot of invalid, expired or Xcode managed profiles/certificates, you can use the `match nuke` command to revoke your certificates and provisioning profiles. Don't worry, apps that are already available in the App Store / TestFlight will still work. Builds distributed via Ad Hoc or Enterprise will be disabled after nuking your account, so you'll have to re-upload a new build. After clearing your account you'll start from a clean state, and you can run `match` to generate your certificates and profiles again. If revoking your certificates is not an option for you, please consider to use match's [import](#import_certificate) feature instead.


Grammar nitpick: "please consider using match's [import]...."

endocrimes · 2017-08-01T13:22:47Z

match/README.md

@@ -358,6 +358,19 @@ If you want to manually decrypt a file you can.
 openssl aes-256-cbc -k "<password>" -in "<fileYouWantToDecryptPath>" -out "<decryptedFilePath>" -a -d
 ```

+### <a name="import_certificate"></a>Import an existing certificate
+
+To quickly and easily get started with `match`, it is adviced to revoke your existing certificates. Although, if revoking code signing certificates is not an available option for you, match provides an `import` feature that let's you import an existing code signing identity certificate, to your match's repository.


couple of grammar/spelling nits:
"it is advised that"
"feature that lets you" (no apostrophe)

endocrimes · 2017-08-01T13:23:16Z

match/README.md

+
+To quickly and easily get started with `match`, it is adviced to revoke your existing certificates. Although, if revoking code signing certificates is not an available option for you, match provides an `import` feature that let's you import an existing code signing identity certificate, to your match's repository.
+
+Before using this feature it is required that the code signing certificate you want to import is properly installed in your Keychain and it is associated with an active code signing identity registered in your Apple Developer account. Check this [fastlane.docs page](<to-do-link-to-fastlane-docs-page>) to easily speed up with this requirements.


Is there a corresponding docs pr?

endocrimes · 2017-08-01T13:25:30Z

match/README.md

+match import "<path-to-p12-code-signing-certificate>" "<p12-password-if-not-empty-string>"
+```
+
+The command will validate the certificate that you want to import and associate it with its Apple Developer's representation. The information retrieved from the Apple Developer account will make sure that the certificate to import is properly processed and analyzed by `match` that finally imports, and encrypts it, in its repository.


"The command will validate the certificate and associate it with the Apple Developer Portal entity. This will ensure that the certificate is correctly processed by match and imported into the repository."?

endocrimes · 2017-08-01T13:36:43Z

match/lib/match/spaceship_ensure.rb

+      end
+
+      return found if found
+      UI.user_error!("The certificate to import can not be associated with any existing certificate in the Apple Developer Portal.")


"We could not find a matching certificate in the Apple Developer Portal"?

endocrimes · 2017-08-01T13:44:10Z

match/lib/match/runner.rb

+      params[:import_certificate] = args[0] if params[:import_certificate].nil?
+      UI.user_error!("Missing path to the certificate to import") if params[:import_certificate].nil?
+
+      params[:import_certificate_password] = args[1] if params[:import_certificate_password].nil?


This currently breaks in the case where you specify import_certficate as an env var, but the password on the command line. Not sure how best to handle this though

@dantoml not sure what do you mean. Anyway, fixed all other comments, thanks! Let me know if you need something else :)

@razum2um Hey, so currently:

if you ran:

MATCH_IMPORT_CERTIFICATE=/path_to_cert fastlane match import $CERTIFICATE_PASSWORD

it would fail to find the password, and could be somewhat confusing. This is however, potentially fine, I was mostly interested to see what your thoughts were and if you'd considered it.

@dantoml I think specifying cert & pass in a different way is a strange choice. Should we really handle it? Maybe refuse positional arguments (but I'm rather not)?

KrauseFx · 2017-08-04T16:57:07Z

match/README.md

+To import your code signing certificate, P12 encoded, run:
+
+```
+match import "<path-to-p12-code-signing-certificate>" "<p12-password-if-not-empty-string>"


Should probably be fastlane match and not just match, right?

@KrauseFx fixed, thanks :)
let me know, what can I do to get this finally merged? ;)

KrauseFx · 2017-08-04T23:21:24Z

Sorry to be so negative on this, but I'm not very comfortable merging this. I can already see this feature causing lots of incoming GitHub issues from people that import invalid certificates and keys, or mismatching profiles, causing more confusion. The beauty of match is that you can be sure things work as expected.

I said the same thing a few times in the past, I know that this is a feature people wish for, but I'm not super comfortable with it, and I don't think I'm the one ready to merge it.

razum2um · 2017-08-05T15:52:10Z

@KrauseFx I understand your worries, features are coming with the cost of supportability :)

What can we do in order to prevent "issues from people that import invalid certificates and keys" - check CNs, expiration dates?
Maybe print more info about supplied certificate as well as found ones on devportal?

Can we turn "so negative on this" to constructive suggestions?

seclace · 2017-08-10T07:10:25Z

@KrauseFx @dantoml Some news about this merge request? It looks very helpful to many.

MajinRaph · 2017-09-27T14:32:47Z

still nothing about this feature ? :)

KrauseFx · 2017-09-27T14:37:54Z

This is still my position: #9752 (comment)

If someone else feels comfortable reviewing, testing, merging and maintaining the code, I'm okay, but I won't sign off, sorry again, but that's my opinion on this feature (has nothing to do with the code)

milch · 2017-09-27T15:09:13Z

Maybe we can find a compromise if we can understand the exact reasons for wanting this. My understanding is the following:

Due to customers or company regulations, it is not possible to give out the credentials that would allow match to create certificates and profiles
At the same time, it is also not possible to run match on the account owner's computer once and then use readonly: true from the developer's computer
- Why?
Anything else?

razum2um · 2017-09-27T19:05:46Z

@matthewellis no, there was no such intention and last time I used it myself I synced dev certs, right, will sort it out as soon as I can

razum2um · 2017-09-27T19:11:09Z

@milch my reason (maybe I've just chosen wrong approach): we have different match repos with different credentials for different projects. We were trying to separate dev certificates as well, but apple restricted the number of total possible dev certificates to 5, so in order to maintain the flow I imported one created earlier into next project's match repo

milch · 2017-10-01T14:54:49Z

@razum2um Yeah, that doesn't sound like how match is intended to be used. The idea is more or less to have 1 match repo per Apple account. Is there a reason why you can't have a single repo for all projects?

MajinRaph · 2017-10-04T07:53:37Z

As for me, I think this feature is needed to prevent legacy enterprise certificates from being revoked by match at first use, as it would disable all the existing apps (unless I'm misunderstanding somehting).

KrauseFx · 2017-10-04T08:12:21Z

You don't necessarily have to nuke, if you still have a free spot for a new certificate, match will just use that 👍

MajinRaph · 2017-10-04T14:32:03Z

@KrauseFx sure, but if we have no spots left, the feature would be useful :P (even if, when we have two certificates, we can nuke the unused (yet) one)

And thank you for your answer (and for Fastlane, which is an amazing toolsuit)

ohayon · 2017-12-04T20:25:54Z

Hey folks! I am going to close this PR for now as it has been inactive for quite some time. That being said, please feel free to continue conversation on here and let us know if we should reopen it at any point! 🚀

janpio · 2017-12-18T21:38:36Z

@milch To add to your list, I learned that revoking an Enterprise cer/profile combination (via nuke) makes the app unusable that moment. So not like with a normal app store cert that you just replace with the next update, your users can't use your app until they get one built with the new cert.

janpio · 2017-12-19T09:59:29Z

As KrauseFx understandable is cautious about adding this to fastlane itself, how about reworking it into a plugin with its own Github repo @razum2um @MajinRaph? Then problems using it would end up at the issues of that repository and one could make sure that all information for this "advanced usage" is collected there.

googlebot added the cla: no label Jul 14, 2017

googlebot added cla: yes and removed cla: no labels Jul 14, 2017

razum2um mentioned this pull request Jul 14, 2017

[match] Import P12 codesigning certificate to match's repo. #6972

Closed

KrauseFx added tool: match type: feature labels Jul 14, 2017

fastlane-bot added the status: needs-attention label Jul 28, 2017

endocrimes requested changes Aug 1, 2017

View reviewed changes

razum2um force-pushed the import-certificate branch from af2f7e4 to a62dc94 Compare August 1, 2017 14:09

ohayon mentioned this pull request Aug 4, 2017

[question] How to keep push notification certificates in shared git repo with match? #8818

Closed

KrauseFx reviewed Aug 4, 2017

View reviewed changes

Match import certificate

8d89cd0

razum2um force-pushed the import-certificate branch from a62dc94 to 8d89cd0 Compare August 4, 2017 18:29

ohayon closed this Dec 4, 2017

fastlane-bot removed the status: needs-attention label Dec 5, 2017

fastlane locked and limited conversation to collaborators Feb 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[match] import P12 codesigning certificate to match's repo, updated, fixed #9752

[match] import P12 codesigning certificate to match's repo, updated, fixed #9752

razum2um commented Jul 14, 2017 •

edited

googlebot commented Jul 14, 2017

razum2um commented Jul 14, 2017

googlebot commented Jul 14, 2017

razum2um commented Jul 14, 2017

ByKeks commented Jul 31, 2017

endocrimes left a comment

endocrimes Aug 1, 2017

endocrimes Aug 1, 2017

endocrimes Aug 1, 2017

endocrimes Aug 1, 2017

endocrimes Aug 1, 2017

endocrimes Aug 1, 2017

razum2um Aug 1, 2017

endocrimes Aug 4, 2017

razum2um Aug 4, 2017 •

edited

KrauseFx Aug 4, 2017

razum2um Aug 4, 2017

KrauseFx commented Aug 4, 2017

razum2um commented Aug 5, 2017 •

edited

seclace commented Aug 10, 2017

MajinRaph commented Sep 27, 2017

KrauseFx commented Sep 27, 2017

milch commented Sep 27, 2017

razum2um commented Sep 27, 2017

razum2um commented Sep 27, 2017

milch commented Oct 1, 2017

MajinRaph commented Oct 4, 2017

KrauseFx commented Oct 4, 2017

MajinRaph commented Oct 4, 2017 •

edited

ohayon commented Dec 4, 2017

janpio commented Dec 18, 2017

janpio commented Dec 19, 2017


		To quickly and easily get started with `match`, it is adviced to revoke your existing certificates. Although, if revoking code signing certificates is not an available option for you, match provides an `import` feature that let's you import an existing code signing identity certificate, to your match's repository.

		Before using this feature it is required that the code signing certificate you want to import is properly installed in your Keychain and it is associated with an active code signing identity registered in your Apple Developer account. Check this [fastlane.docs page](<to-do-link-to-fastlane-docs-page>) to easily speed up with this requirements.

[match] import P12 codesigning certificate to match's repo, updated, fixed #9752

[match] import P12 codesigning certificate to match's repo, updated, fixed #9752

Conversation

razum2um commented Jul 14, 2017 • edited

Checklist

Motivation and Context

Description

googlebot commented Jul 14, 2017

razum2um commented Jul 14, 2017

googlebot commented Jul 14, 2017

razum2um commented Jul 14, 2017

ByKeks commented Jul 31, 2017

endocrimes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

razum2um Aug 4, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KrauseFx commented Aug 4, 2017

razum2um commented Aug 5, 2017 • edited

seclace commented Aug 10, 2017

MajinRaph commented Sep 27, 2017

KrauseFx commented Sep 27, 2017

milch commented Sep 27, 2017

razum2um commented Sep 27, 2017

razum2um commented Sep 27, 2017

milch commented Oct 1, 2017

MajinRaph commented Oct 4, 2017

KrauseFx commented Oct 4, 2017

MajinRaph commented Oct 4, 2017 • edited

ohayon commented Dec 4, 2017

janpio commented Dec 18, 2017

janpio commented Dec 19, 2017

razum2um commented Jul 14, 2017 •

edited

razum2um Aug 4, 2017 •

edited

razum2um commented Aug 5, 2017 •

edited

MajinRaph commented Oct 4, 2017 •

edited