Skip to content
This repository was archived by the owner on Jun 5, 2025. It is now read-only.

Vcert-java initial implementation#2

Merged
mr-tron merged 32 commits intoVenafi:masterfrom
opencredo:master
May 28, 2019
Merged

Vcert-java initial implementation#2
mr-tron merged 32 commits intoVenafi:masterfrom
opencredo:master

Conversation

@pegerto
Copy link
Copy Markdown
Contributor

@pegerto pegerto commented Mar 28, 2019

No description provided.

pegerto and others added 7 commits March 21, 2019 11:25
Adds authorization functionality for Cloud and Onprem
* adds Read Zone Configuration functionality
* adds Wiremock for testing
* Register an user venafi cloud

* Add target to gitingnore
* Search cloud certificates

* Optimize imports
@pegerto
Copy link
Copy Markdown
Contributor Author

pegerto commented Mar 29, 2019

Hi @tr1ck3r

could you take a look at the current progress, we are progressing on the development, we can add any suggestion base on your input.

pmack24 added 2 commits April 3, 2019 18:23
* Certificate request pojo to be used for creating and retrieving certs
* Implement connector generate certificate request
* Generate CSR for RSA and ECDSA
* Uncomment connector interface method and throw exceptions for yet to be implemented methods
* Add integration tests to verify phase
* Feature/request certificate cloud (#4)
* migrates request certificate method for cloud
* adds tests for request certificate for cloud
@mr-tron
Copy link
Copy Markdown
Contributor

mr-tron commented Apr 4, 2019

You can remove register. It`s deprecated function.

assertThat(zoneConfiguration.province()).isNull();
assertThat(zoneConfiguration.locality()).isNull();
assertThat(zoneConfiguration.policy()).isNotNull();
assertThat(zoneConfiguration.policy().subjectCNRegexes()).containsExactly(".*");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i don't know how and what type of regular expressions dialect do you use, but remember, that regexp should match full string. so for example regexp .*\.example\.com should doesn't match cn www.example.company

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hello I see, in this case, the test is validating that the API requires its correct from a stub external service https://github.com/opencredo/vcert-java-1/blob/68f1d07aaf4fc61e613361f68fb43b5932712cea/src/test/resources/mappings/cloud.certificatepolicies.di.json#L40

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I dont big specialist in java. I didt know what java pattern.matches which match whole string. In golang you need to add ^ and $ to your regexp for matching whole string.

@mr-tron
Copy link
Copy Markdown
Contributor

mr-tron commented Apr 4, 2019

Also please move hardcoded tokens and urls in tests to environment variables.

pmack24 and others added 11 commits April 4, 2019 12:40
* Retrieve certificate for cloud
* Retrieve certificate for tpp
* migrates request certificate method for cloud
* adds tests for request certificate for cloud

* Feature/request certificate tpp (#5)

* migrates request certificate method for tpp
* adds test for request certificate for tpp
Not supported by cloud
Renew a cloud certificate
@pegerto pegerto changed the title Bootstrap and Autorisation Vcert-java initial implementation Apr 5, 2019
@pegerto
Copy link
Copy Markdown
Contributor Author

pegerto commented Apr 5, 2019

You can remove register. It`s deprecated function.

Done

@mr-tron
Copy link
Copy Markdown
Contributor

mr-tron commented Apr 5, 2019

As I understand you just rewrite golang code to java. We are going to make some changes in the nearest future. May be you can just write better code from beginning and not copy our errors.

    certificate.Request has unused public attribute PublicKeyAlgorithm - should be removed (duplicated by KeyType)
    certificate.Request has public field CSR []byte what make misunderstoonding what can bein this field: pem or asn encoded csr. Should be replaced with private filed csr and few methods like SetCSR (may be with auto detect type), GetCSRasn1, GetCSRpem
    few methods accept empty interface as private key. should be replaced with crypto.Signer interface
    rewrite retrive function for cloud connector. should be getting certID by cn/thumbprint/pickupid -> getting certificate by certID via /v1/certificate/ {id}
    `func (cfg *Config) LoadFromFile() error` should be replaced with `func LoadConfigFromFile(path) (*Config, error)`
    SetBaseURL should be replaced with additional argument in NewConnector function //discuss

@pegerto
Copy link
Copy Markdown
Contributor Author

pegerto commented Apr 5, 2019

Hello @M-Tron

I fully understand we discuss this in our proposal, it will be good to have a more fluent API and refactor sections on our code, unfortunately, we are time constrained, so we implement as match parity as possible for tracking.

We will try to implement part of the refactoring in this initial PR.

@pegerto
Copy link
Copy Markdown
Contributor Author

pegerto commented Apr 10, 2019

Also please move hardcoded tokens and urls in tests to environment variables.

we have externalized acceptance test so it can be run by an integration environment and document the variables in the README file

@mr-tron
Copy link
Copy Markdown
Contributor

mr-tron commented Apr 26, 2019

Can you also add example of library usages like https://github.com/Venafi/vcert/blob/master/example/main.go with

  1. enrolling certificate
  2. retrieving certificate
  3. renew
  4. checking certificate request with zone policies (I think CN will be enough)

@pegerto
Copy link
Copy Markdown
Contributor Author

pegerto commented May 7, 2019

@mr-tron an Example class has been added this PR, The recommendation is to create a different project for examples, or submodule the project if more examples are required.

Include additional examples (fix key and examples) (#26)
@mr-tron mr-tron merged commit a006b33 into Venafi:master May 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants