#282 very basic implementation of custom specified path for idp certificate
Closed by t0xic0der. Opened by webrat.
webrat/ipsilon master  into  master

Download 282.patch

things one might want to add:
- check if file exists
- check if file is correctly encoded
- test in general

maybe fixes https://pagure.io/ipsilon/issue/19

2 new commits added

  • Merge branch 'master' of ssh://pagure.io/forks/webrat/ipsilon
  • very basic implementation of custom specified path

If the user provides --saml2-cert-path but not --saml2-key-path, this will raise a KeyError. It might be nice to add something to validate that providing one requires the other, with a friendly error message when there's a problem so the traceback is avoided.

I recommend tests, but LGTM.

1 new commit added

  • add a little bit of opts checking

added a little bit of checking.
wasn't able to get the test env running here @puiterwijk probably wanted to add some tests

1 new commit added

  • remove double import of sys

Is there a specific reason you don't use Certificate.import_cert, which would set both properties with a single call?

Sorry for the delay in reviewing this. I've commented with a single question, but I'm not sure that this is enough.
The code should probably have an import_external_cert function that will set the self.cert and self.key, but also copy that cert and key to the install path.
The problem if you don't do that is that you have no idea where the certificate might be stored, so the user setting it up (root) might have access to it, but the ipsilon user might not, either due to file ACLs or selinux.

Pull-Request has been closed by t0xic0der

Metadata