Update To Support v5 API #48

JakeQuilty · 2020-05-15T21:05:51Z

This PR updates the .NET API to support the v5 API.

Functionality Added

Update support from v4 to v5
Update README to show functionality of this API
Added option to control "limit" and "offset" for ListUsers() and ListVariables()

These methods were tested to make sure they support the v5 API. More details

Methods

Client

Client Client(uri, account)

Create new Conjur instance
- uri - URI of the Conjur server. Example: https://myconjur.org.com
- account - Name of the Conjur account

LogIn(string userName, string password)

Login to a Conjur user
- userName - Username of Conjur user to login as
- password - Passwordof user

TrustedCertificates.ImportPem (string certPath)

Add Conjur root certificate to system trust store
- certPath = Path to cert

<Client>.Credential = new NetworkCredential(string userName, string apiKey)

To login with an API key, use it directly
- userName - Username of user to login as
- apiKey - API key of user

IEnumerable<Variable> ListVariables(string query = null)

Returns a list of variable objects
- name="query" - Additional Query parameters, not required

uint CountVariables(string query = null)

Count Conjur resource of kind variable
- name="query" - Additional Query parameters, not required

Host CreateHost(string name, string hostFactoryToken)

Creates a host using a host factory token
- name - Name of the host to create
- hostFactoryToken - Host factory token

Policy

Policy <Client>.Policy(string policyName)

Create a Conjur policy object
- policyName - Name of policy

policy.LoadPolicy(Stream policyContent)

Load policy into Conjur
- policyContent - The policy

Variable

Variable <Client>.Variable(string name)

Instantiate a Variable object
- name - Name of the variable

Boolean Check(string privilege)

Check if the current user has specified privilege on this variable
- privilege - string name of the privilege to check for
  - Privileges: read, create, update, delete, execute

AddSecret(string val)

Change current Variable to val
- val - Value to update current Variable to

String GetValue()

Return the value of the current Variable

Tests Fixed

api-dotnet.test.Conjur.Test.VariablesTest.GetVariableTest

Error: System.Collections.Generic.KeyNotFoundException : test:///secrets/test-account/variable/foo bar
Uses Uri.EscapeDataString
Changed + to %20

api-dotnet.test.Conjur.Test.UserTest.ListUserTest
api-dotnet.test.Conjur.Test.VariablesTest.ListVariableTest

Error: System.Collections.Generic.KeyNotFoundException : test:///resources/test-account/variable?offset=0&limit=10000
Both list tests have limit=10000 that's not in expected URIs
verify<>Info is the same for both
Test was assuming that offset and limit were variable in the API. They are static.
ListResources at the lowest level was not static, BUT for listing resources at the Variable and User level, there was no parameter that would go all the way down to Resources.ListResources to make limit and offset dynamic
Tests do not account for the multiple calls the ListResources method makes.
ListResources will make calls until the returned call has a count of 0, so a second call will be made that has a count of 0 for these tests but with an offset equal to limit

api-dotnet.test.Conjur.Test.ClientTest.ActingAsTest

Similar problem to ListUser and ListVariable of the default limit did not match the specified limit in the test

Outstanding Work

Tests to Fix

api-dotnet.test.Conjur.Test.AuthenticatorTest.TestTokenCaching
api-dotnet.test.Conjur.Test.AuthenticatorTest.TestTokenThreadSafe
api-dotnet.test.Conjur.Test.ClientTest.TestLogin

These 3 all use GetToken() which uses HttpWebRequest request = WebRequest.CreateHttp(this.uri), and it's not configured in WebMocker.
Cannot override the CreateHttp method to return a mocked HttpWebRequest object because "cannot override inherited member, because it is not marked abstract, virtual or override"
Issue: Fix Failing Tests Using GetToken() #45

api-dotnet.test.Conjur.Test.HostFactoryTest.TestCreateHost
api-dotnet.test.Conjur.Test.ResourceTest.TestCheck

System.TypeLoadException
Issue: Fix Failing Tests "System.TypeLoadException" #46

Functionality to Add

Although you can get manually get the environment variables and assign them to the needed variables to instantiate a Client, it would be nice if there was a built in way to do this like the Java API
- Issue: Retrieving Environment Variables for Authentication #47
Adding batch secret retrieval as a feature of The API, so it doesn't need to be done in manually by user
- Issue: Add Batch Secret Retrieval #49

sgnn7

@JakeQuilty I did what I could to CR it. Let me know if you have any questions.

LICENSE

README.md

test/AuthenticatorTest.cs

test/Base.cs

test/ClientTest.cs

test/HostFactoryTest.cs

test/ResourceTest.cs

sgnn7 · 2020-05-18T20:14:16Z

@JakeQuilty Also make sure to update the wording of the "update to dev" commit since it seems really slim and un-useful

The ListVariables method did not have the option to specify limit and offset parameters. The Resource method that parented it did have this parameter, and the original tests accounted for it. I also fixed the tests not accounting for an extra API call that the ListResources method makes.

This commit has an identical solution to the ListVariables. See the ListVariables commit for more details.

JakeQuilty · 2020-05-20T18:46:01Z

@uCatu can you take a look at some of @sgnn7 's suggestions to your commit?

conjur-api/Client.Methods.cs

uCatu · 2020-05-20T19:47:21Z

conjur-api/ApiConfigurationManager.cs

+            {
+                if (httpRequestTimeout == null)
+                {
+                    lock (locker)


I don't know if it's the best pattern but we are locking a singleton so it looks good
@sashaCher - adding as a reviewer

uCatu · 2020-05-20T19:51:38Z

conjur-api/ApiConfigurationManager.cs

+                        {
+                            string value = ConfigurationManager.AppSettings.Get(HTTP_REQUEST_TIMEOUT);
+                            httpRequestTimeout = 100000; //100 seconds; WebRequest default timeout
+                            if (!string.IsNullOrWhiteSpace(value))


We can extract a method that get values from configuration or use default and use it twice in thie class

uCatu · 2020-05-20T19:55:03Z

conjur-api/ApiKeyAuthenticator.cs

-                Interlocked.Exchange(ref this.token, request.Read());
-                this.StartTokenTimer(new TimeSpan(0, 7, 30));
+                    IntPtr bstr = IntPtr.Zero;
+                    byte[] bArr = new byte[credential.SecurePassword.Length];


We can add - cleaning secrets from memory footprint :)

conjur-api/ApiKeyAuthenticator.cs

conjur-api/Client.Methods.cs

sgnn7

@JakeQuilty Just a few more comments

README.md

sgnn7 · 2020-05-20T21:32:53Z

conjur-api/ApiConfigurationManager.cs

@@ -0,0 +1,103 @@
+// <copyright file="ApiConfigurationManager.cs" company="Conjur Inc.">
+//     Copyright (c) 2016 Conjur Inc. All rights reserved.


All files still need copyright section updated

sgnn7 · 2020-05-20T21:36:35Z

conjur-api/ApiConfigurationManager.cs

+            {
+                if (httpRequestTimeout == null)
+                {
+                    lock (locker)


II read some more on this - I think that due to this being an int, you can just mark it as volatile instead of locking.

conjur-api/ApiKeyAuthenticator.cs

conjur-api/Client.Methods.cs

conjur-api/Resource.cs

conjur-api/User.cs

JakeQuilty · 2020-05-22T17:11:10Z

All cosmetic changes are done. The last commit is failing to build for the tests though

sgnn7 · 2020-05-26T18:26:26Z

@JakeQuilty / @uCatu Can we figure out how to get the current tests passing so that we can get this merged?

sgnn7 · 2020-05-26T18:27:38Z

Current errors:

[2020-05-22T17:11:00.545Z] /build/conjur-api/conjur-api.csproj (default targets) ->
[2020-05-22T17:11:00.545Z] /usr/lib/mono/4.5/Microsoft.CSharp.targets (CoreCompile target) ->
[2020-05-22T17:11:00.545Z] 
[2020-05-22T17:11:00.545Z] 	Client.cs(66,16): error CS1043: Invalid accessor body `=>', expecting `;' or `{'
[2020-05-22T17:11:00.545Z] 	Client.cs(66,45): error CS1014: A get or set accessor expected
[2020-05-22T17:11:00.545Z] 	ApiConfigurationManager.cs(49,16): error CS1043: Invalid accessor body `=>', expecting `;' or `{'
[2020-05-22T17:11:00.545Z] 	ApiConfigurationManager.cs(73,16): error CS1043: Invalid accessor body `=>', expecting `;' or `{'
[2020-05-22T17:11:00.545Z] 
[2020-05-22T17:11:00.545Z] 	 0 Warning(s)
[2020-05-22T17:11:00.545Z] 	 4 Error(s)

izgeri · 2020-05-26T18:52:56Z

@sgnn7 @JakeQuilty is this even with some of the tests commented out, to be updated in follow-up issues? or are we not commenting out any test cases anymore?

I am ok with commenting some test cases and fixing them in follow-up issues, as long as we log the issues for them and reference them here

sgnn7 · 2020-05-26T19:18:46Z

@izgeri the failures are with some tests already commented out. As far as I can tell, the project cannot build.

sashaCher · 2020-05-26T19:34:20Z

@sgnn7 The origin of those error is C#7 syntax.
In our environment we are building the SDK with up to date msbuild toolset so we even haven't pay an attention to this.
You can read more about syntax and versions there.
You can "rephrase" those code segments using C#6 or lower convention or to upgrade mono compiler. It looks that it supports "Expression-bodied constructors, finalizers and accessors".

sgnn7 · 2020-05-26T19:46:07Z

@sashaCher Thanks for the explanation - makes sense! It may be useful to use the old syntax until we figure out what the bottom version for this API should be though but feel free to update the test runner instead if C#6 is end-of-life already.

JakeQuilty · 2020-05-27T19:12:58Z

Fixed the failing build by reverting all the get and set with => back to curly brackets

build.sh was failing to build due to different the code being updated with C#7 syntax instead of C#6. More detail [HERE](#48 (comment))

build.sh was failing to build due to different the code being updated with C#7 syntax instead of C#6. This commit reverts those syntax changes. More detail #48 (comment)

sgnn7

I noticed some nits but at this point we need to get this code into the repo and we can improve afterwards.

Outdated

- README: - Removed old information - Added table of contents, required versions, contributing link - CONTRIBUTING: - Updated to follow community standards - Added some testing and dev info - CHANGELOG: - Added changes from #48 - Added links to older releases

JakeQuilty mentioned this pull request May 15, 2020

Master branch supports Conjur OSS and DAP #43

Closed

2 tasks

JakeQuilty requested a review from izgeri May 15, 2020 21:33

JakeQuilty marked this pull request as ready for review May 15, 2020 21:33

JakeQuilty linked an issue May 18, 2020 that may be closed by this pull request

Master branch supports Conjur OSS and DAP #43

Closed

2 tasks

sgnn7 suggested changes May 18, 2020

View reviewed changes

JakeQuilty force-pushed the ca-conjur-api-dotnet-v5-master branch from 3fa0bbe to 32dabdd Compare May 20, 2020 17:43

uCatu and others added 6 commits May 20, 2020 14:41

Update to support v5 API

cffdc3d

Update README

32b2549

Fix ListUsers not having limit/offset parameters and tests

8589fca

This commit has an identical solution to the ListVariables. See the ListVariables commit for more details.

Comment out leftover failing tests

5490b16

Add parse changelog to fix failing Jenkins test

f29096a

JakeQuilty force-pushed the ca-conjur-api-dotnet-v5-master branch from 32dabdd to c8ad39f Compare May 20, 2020 18:41

JakeQuilty force-pushed the ca-conjur-api-dotnet-v5-master branch 2 times, most recently from 163b7c0 to 244e44c Compare May 20, 2020 18:56

uCatu previously requested changes May 20, 2020

View reviewed changes

uCatu requested a review from sashaCher May 20, 2020 19:59

sgnn7 reviewed May 20, 2020

View reviewed changes

JakeQuilty and others added 2 commits May 22, 2020 13:09

Cosmetic fixes from suggestions

197518a

stylecop & license update

a120722

JakeQuilty force-pushed the ca-conjur-api-dotnet-v5-master branch from e09bb94 to a120722 Compare May 22, 2020 17:10

JakeQuilty pushed a commit that referenced this pull request May 27, 2020

Fix failures in build.sh

785771f

build.sh was failing to build due to different the code being updated with C#7 syntax instead of C#6. More detail [HERE](#48 (comment))

JakeQuilty force-pushed the ca-conjur-api-dotnet-v5-master branch from 8853b17 to 785771f Compare May 27, 2020 19:23

Fix failures in build.sh

ca74f0f

build.sh was failing to build due to different the code being updated with C#7 syntax instead of C#6. This commit reverts those syntax changes. More detail #48 (comment)

JakeQuilty force-pushed the ca-conjur-api-dotnet-v5-master branch from 785771f to ca74f0f Compare May 27, 2020 19:25

sgnn7 approved these changes May 27, 2020

View reviewed changes

sgnn7 merged commit 0e6432f into master May 27, 2020

sgnn7 deleted the ca-conjur-api-dotnet-v5-master branch May 27, 2020 19:56

JakeQuilty mentioned this pull request May 28, 2020

Repo Meets Community Baseline Requirements #60

Closed

8 tasks

JakeQuilty mentioned this pull request May 28, 2020

Update documentation for V5 support #61

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update To Support v5 API #48

Update To Support v5 API #48

JakeQuilty commented May 15, 2020 •

edited

Loading

sgnn7 left a comment

sgnn7 commented May 18, 2020

JakeQuilty commented May 20, 2020

uCatu May 20, 2020

uCatu May 20, 2020

uCatu May 20, 2020

sgnn7 left a comment

sgnn7 May 20, 2020

sgnn7 May 20, 2020

JakeQuilty commented May 22, 2020

sgnn7 commented May 26, 2020

sgnn7 commented May 26, 2020

izgeri commented May 26, 2020

sgnn7 commented May 26, 2020

sashaCher commented May 26, 2020

sgnn7 commented May 26, 2020

JakeQuilty commented May 27, 2020

sgnn7 left a comment

		@@ -0,0 +1,103 @@
		// <copyright file="ApiConfigurationManager.cs" company="Conjur Inc.">
		// Copyright (c) 2016 Conjur Inc. All rights reserved.

Update To Support v5 API #48

Update To Support v5 API #48

Conversation

JakeQuilty commented May 15, 2020 • edited Loading

Functionality Added

Methods

Client

Policy

Variable

Tests Fixed

Outstanding Work

Tests to Fix

Functionality to Add

sgnn7 left a comment

Choose a reason for hiding this comment

sgnn7 commented May 18, 2020

JakeQuilty commented May 20, 2020

uCatu May 20, 2020

Choose a reason for hiding this comment

uCatu May 20, 2020

Choose a reason for hiding this comment

uCatu May 20, 2020

Choose a reason for hiding this comment

sgnn7 left a comment

Choose a reason for hiding this comment

sgnn7 May 20, 2020

Choose a reason for hiding this comment

sgnn7 May 20, 2020

Choose a reason for hiding this comment

JakeQuilty commented May 22, 2020

sgnn7 commented May 26, 2020

sgnn7 commented May 26, 2020

izgeri commented May 26, 2020

sgnn7 commented May 26, 2020

sashaCher commented May 26, 2020

sgnn7 commented May 26, 2020

JakeQuilty commented May 27, 2020

sgnn7 left a comment

Choose a reason for hiding this comment

JakeQuilty commented May 15, 2020 •

edited

Loading