Add apikey authenticator #46

nan2iz · 2021-10-12T05:42:19Z

How to call the authenticator:

CloudPakForDataAuthenticator authenticator = new CloudPakForDataAuthenticator()
                .WithUrl($CLUSTER_URL)
                .WithUserName($USER_NAME)
                .WithApikey($API_KEY)
                .WithDisableSslVerification($DISABLE_SSL)
                .Build();
            service = new AssistantService(versionDate, authenticator);
            service.DisableSslVerification($DISABLE_SSL);
            service.SetServiceUrl($SERVICE_URL);

…te to use /v1/authorize

mediumTaj

just some questions after a quick readthrough nan

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataTokenResponse.cs

kevinkowa

It looks good, just a comment

kevinkowa · 2021-10-12T15:55:58Z

test/IBM.Cloud.SDK.Core.Tests/CloudPak4DataAuthTests.cs

+        }
+
+        [TestMethod]
+        public void TestConstructionApikeyRequried()


possible typo on the name of the method

Ok. Will fix it. Thank you

mediumTaj · 2021-10-13T20:53:35Z

Also need to check on CI here

mediumTaj · 2021-10-18T15:02:39Z

appveyor.yml

@@ -17,6 +17,7 @@ environment:
        secure: gevsMhy8RTWMdf7MjOnIo5QaN6JpL9DPK6I+Go5ByZir5LDwyFsv9hO6nBjGTg8n #pragma: whitelist secret
    GITHUB_TOKEN:
        secure: +B2bs86RVtJtlbkB+cTf9bkqnNlFJi/PbBBPzR5jlUlLLZoOc+ZgqgQLwee4tCT+ #pragma: whitelist secret
+    nodejs_version: 15.6.0


FYI i made this change in another PR for core so it matches the .NET SDK node version
b422967

Okay. I will remove this one. Thank you

mediumTaj · 2021-10-18T15:23:53Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+        /// <param name="headers">A set of user-supplied headers to be included in token server interactions</param>
+        public CloudPakForDataAuthenticator(string url, string username, string password, string apikey, bool? disableSslVerification = null, Dictionary<string, string> headers = null)
+        {
+            Init(url, username, password, apikey, disableSslVerification, headers);


@nan2iz did you get anywhere with the builder/fluent pattern for this?

I think if the builder pattern works, I dont think we need the 2nd overload.

Okay. I will remove the overload. Thank you

mediumTaj

I threw some suggestions in here for consistency

mediumTaj · 2021-10-18T17:57:49Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+        /// <param name="headers">A set of user-supplied headers to be included in token server interactions</param>
+        public CloudPakForDataAuthenticator(string url, string username, string password, string apikey, bool? disableSslVerification = null, Dictionary<string, string> headers = null)
+        {
+            Init(url, username, password, apikey, disableSslVerification, headers);


I think if the builder pattern works, I dont think we need the 2nd overload.

mediumTaj · 2021-10-18T18:02:45Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+
+        public CloudPakForDataAuthenticator SetUrl(string url)
+        {
+            this.Url = url;


Suggested change

this.Url = url;

Url = url;

Thank you. Will fix it

mediumTaj · 2021-10-18T18:03:00Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+
+        public CloudPakForDataAuthenticator SetUsername(string username)
+        {
+            this.Username = username;


Suggested change

this.Username = username;

Username = username;

mediumTaj · 2021-10-18T18:03:16Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

        }

-        private void Init(string url, string username, string password, bool? disableSslVerification = null, Dictionary<string, string> headers = null)
+        public CloudPakForDataAuthenticator setPassword(string password)


Suggested change

public CloudPakForDataAuthenticator setPassword(string password)

public CloudPakForDataAuthenticator SetPassword(string password)

mediumTaj · 2021-10-18T18:03:34Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

-        private void Init(string url, string username, string password, bool? disableSslVerification = null, Dictionary<string, string> headers = null)
+        public CloudPakForDataAuthenticator setPassword(string password)
+        {
+            this.Password = this.Password;


Suggested change

this.Password = this.Password;

Password = password;

mediumTaj · 2021-10-18T18:03:49Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+            return this;
+        }
+
+        public CloudPakForDataAuthenticator setApikey(string apikey)


Suggested change

public CloudPakForDataAuthenticator setApikey(string apikey)

public CloudPakForDataAuthenticator SetApikey(string apikey)

mediumTaj · 2021-10-18T18:04:07Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+
+        public CloudPakForDataAuthenticator setApikey(string apikey)
+        {
+            this.Apikey = this.Apikey;


Suggested change

this.Apikey = this.Apikey;

Apikey = apikey;

mediumTaj · 2021-10-18T18:05:39Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+
+        public CloudPakForDataAuthenticator SetDisableSslVerification(bool disableSslVerification)
+        {
+            this.DisableSslVerification = disableSslVerification;


Suggested change

this.DisableSslVerification = disableSslVerification;

DisableSslVerification = disableSslVerification;

mediumTaj · 2021-10-18T18:06:04Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+
+        public CloudPakForDataAuthenticator SetHeaders(Dictionary<string, string> headers)
+        {
+            this.Headers = headers;


Suggested change

this.Headers = headers;

Headers = headers;

nan2iz · 2021-10-19T17:01:01Z

Note: I will need to wait until this PR #47 is merged to the master first.
Since it will fix the build issue. And then I will pull into my branch.

kevinkowa

Looks good!

…-authenticator

mediumTaj · 2021-10-20T20:42:02Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

        /// <param name="username">The username to be used when retrieving the access token</param>
        /// <param name="password">The password to be used when retrieving the access token</param>
        /// <param name="disableSslVerification">A flag indicating whether SSL hostname verification should be disabled</param>
        /// <param name="headers">A set of user-supplied headers to be included in token server interactions</param>
        public CloudPakForDataAuthenticator(string url, string username, string password, bool? disableSslVerification = null, Dictionary<string, string> headers = null)
        {
-            Init(url, username, password, disableSslVerification, headers);
+            Init(url, username, password, null, disableSslVerification, headers);


Do we still need apikey in init?

mediumTaj · 2021-10-20T20:46:19Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+                {
+                    KeyValuePair<string, string> apikey = new KeyValuePair<string, string>(KeyApikey, Apikey);
+                    content.Add(apikey);
+                } else


ah this else should be on a separate line

Suggested change

} else

}

else

…r which use Dictinary config

… http post to use json type

mediumTaj

One comment then i think it looks good!

mediumTaj · 2021-10-22T20:25:26Z

src/IBM.Cloud.SDK.Core/Authentication/CloudPakForData/CloudPakForDataAuthenticator.cs

+            Init();
+        }
+
+        private void Init(string url, string username, string password, string apikey, bool? disableSslVerification = null, Dictionary<string, string> headers = null)


Do we need default values for password and apikey here? I dont think its guaranteed the values will come back from the config

Suggested change

private void Init(string url, string username, string password, string apikey, bool? disableSslVerification = null, Dictionary<string, string> headers = null)

private void Init(string url, string username, string password = null, string apikey = null, bool? disableSslVerification = null, Dictionary<string, string> headers = null)

…null

mediumTaj

👍 Looks good Thank you Nan!!

nan2iz added 2 commits October 11, 2021 21:06

feat(cpd-authentication): add apikey for authentication step and upda…

d92e59b

…te to use /v1/authorize

test(cpd-tests): add testsing for CloudPakAuthentication

8a851ae

nan2iz requested a review from kevinkowa October 12, 2021 05:42

mediumTaj reviewed Oct 12, 2021

View reviewed changes

fix(detect-secrets): update detect-secrets for false positive

d49dcfd

kevinkowa reviewed Oct 12, 2021

View reviewed changes

nan2iz added 3 commits October 15, 2021 13:43

fix(node-version): update appveyor.yml to use node version 15.6.0

60fb13f

fix(renaming): rename function and vararaiables

4c1c026

chore(detect-secrets): ran audit for detect-secrets

f4399a4

mediumTaj reviewed Oct 18, 2021

View reviewed changes

mediumTaj suggested changes Oct 18, 2021

View reviewed changes

nan2iz added 2 commits October 18, 2021 16:34

fix(cp4d): fixed setter method with the correct naming

d3291fe

test(unit-test): remove unused tests

5e61544

kevinkowa approved these changes Oct 19, 2021

View reviewed changes

nan2iz added 2 commits October 20, 2021 08:09

test(unit-testing): fix unit testing and remove unused testing function

055eef4

Merge branch 'main' of github.com:IBM/dotnet-sdk-core into add-apikey…

55e88e9

…-authenticator

mediumTaj reviewed Oct 20, 2021

View reviewed changes

nan2iz added 5 commits October 20, 2021 18:43

fix(cpd-constuctor): revert the code for ConstructionDictionary

b955890

fix(cpd-authenticator): update apikey for CloudPakForDataAuthenticato…

65502b0

…r which use Dictinary config

fix(semantic-release): update semantic-release version

11f3c02

fix(cpd-authenticator): renaming builder pattern functions and update…

0ca8e45

… http post to use json type

fix(cpd-authenticate): remove unused condition

14691dd

mediumTaj suggested changes Oct 22, 2021

View reviewed changes

nan2iz added 2 commits October 22, 2021 13:35

fix(cpd-authentication): fix init function for initalize with password

18b32e4

fix(cpd-authenticator): add default value for password and apikey as …

cb67218

…null

mediumTaj approved these changes Oct 22, 2021

View reviewed changes

nan2iz merged commit 4c1ce73 into main Oct 22, 2021

nan2iz deleted the add-apikey-authenticator branch October 22, 2021 20:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add apikey authenticator #46

Add apikey authenticator #46

nan2iz commented Oct 12, 2021 •

edited

Loading

mediumTaj left a comment

kevinkowa left a comment

kevinkowa Oct 12, 2021

nan2iz Oct 13, 2021

mediumTaj commented Oct 13, 2021

mediumTaj Oct 18, 2021

nan2iz Oct 18, 2021

mediumTaj Oct 18, 2021

mediumTaj Oct 18, 2021

nan2iz Oct 18, 2021

mediumTaj left a comment

mediumTaj Oct 18, 2021

mediumTaj Oct 18, 2021

nan2iz Oct 18, 2021

mediumTaj Oct 18, 2021

mediumTaj Oct 18, 2021

mediumTaj Oct 18, 2021

mediumTaj Oct 18, 2021

mediumTaj Oct 18, 2021

mediumTaj Oct 18, 2021

mediumTaj Oct 18, 2021

nan2iz commented Oct 19, 2021

kevinkowa left a comment

mediumTaj Oct 20, 2021

mediumTaj Oct 20, 2021

mediumTaj left a comment

mediumTaj Oct 22, 2021

mediumTaj left a comment

	public CloudPakForDataAuthenticator setPassword(string password)
	public CloudPakForDataAuthenticator SetPassword(string password)

	public CloudPakForDataAuthenticator setApikey(string apikey)
	public CloudPakForDataAuthenticator SetApikey(string apikey)

	this.DisableSslVerification = disableSslVerification;
	DisableSslVerification = disableSslVerification;

	private void Init(string url, string username, string password, string apikey, bool? disableSslVerification = null, Dictionary<string, string> headers = null)
	private void Init(string url, string username, string password = null, string apikey = null, bool? disableSslVerification = null, Dictionary<string, string> headers = null)

Add apikey authenticator #46

Add apikey authenticator #46

Conversation

nan2iz commented Oct 12, 2021 • edited Loading

mediumTaj left a comment

Choose a reason for hiding this comment

kevinkowa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mediumTaj commented Oct 13, 2021

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

mediumTaj 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nan2iz commented Oct 19, 2021

kevinkowa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mediumTaj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mediumTaj left a comment

Choose a reason for hiding this comment

nan2iz commented Oct 12, 2021 •

edited

Loading