[feature]Adding JSON structure with reference to Question api initial… #43

bhavya-shukla-lrn · 2023-01-23T09:25:16Z

…ization object[LRN-38974]

markus-liang-lrn · 2023-01-31T09:54:41Z

LearnositySDK/Request/Init.cs

            this.validSecurityKeys = new string[5] { "consumer_key", "domain", "timestamp", "expires", "user_id" };
            this.validServices = new string[7] { "assess", "author", "data", "events", "items", "questions", "reports" };
-            this.algorithm = "sha256";
+            this.algorithm = "hmac256";


the algorithm name is hmac-sha256

markus-liang-lrn · 2023-01-31T09:55:40Z

LearnositySDK/Request/Init.cs

+        /// <param name="value"></param>
        /// <returns>The hashed string</returns>
-        private string hashValue(string[] value)
+        private string hashValue(string prehash, string value )


I think the parameter "value" should be "secret" to make it more intuitive

markus-liang-lrn · 2023-01-31T10:13:51Z

LearnositySDK/Request/Init.cs

                        signatureList.Add(this.securityPacket.getString("user_id"));
                        signatureList.Add(this.secret);

-                        string signature = this.hashValue(signatureList.ToArray());


I see the signatureList is only here, so maybe the declaration and the initialisation should be removed too?

markus-liang-lrn · 2023-01-31T10:16:38Z

LearnositySDK/Request/Init.cs

                            {
                                string user_id = users[i];
-                                hashedUsers.set(user_id, Tools.hash(this.algorithm, user_id + this.secret));
+                                hashedUsers.set(user_id, Tools.hash(this.algorithm,this.prehashString, user_id + this.secret));


need space after comma

markus-liang-lrn · 2023-01-31T10:18:12Z

LearnositySDK/Utils/CryptoUtil.cs

+        /// </summary>
+        /// <param name="s">String to be hashed</param>
+        /// <returns>64-character hex string</returns>
+        public static string hmac256(string prehash,string value)


We should use a proper algorithm name, its hmac sha256

markus-liang-lrn · 2023-01-31T10:19:16Z

LearnositySDK/Utils/CryptoUtil.cs

+        /// </summary>
+        /// <param name="s">String to be hashed</param>
+        /// <returns>64-character hex string</returns>
+        public static string hmac256(string prehash,string value)


the value should be secret

markus-liang-lrn · 2023-01-31T10:21:29Z

LearnositySDK/Utils/Tools.cs

+        /// <summary>
+        /// Returns HMAC256 hash
+        /// </summary>
+        /// <param name="prehash"></param>


need explanation of the parameters

markus-liang-lrn · 2023-01-31T10:24:19Z

LearnositySDK/Utils/Tools.cs

+        /// <param name="value"></param>
        /// <returns></returns>
-        public static string hash(string algorithm, string message)
+        public static string hash(string algorithm, string prehash, string value)


bhavya-shukla-lrn · 2023-01-31T11:56:08Z

LearnositySDK/Request/Init.cs

            this.validSecurityKeys = new string[5] { "consumer_key", "domain", "timestamp", "expires", "user_id" };
            this.validServices = new string[7] { "assess", "author", "data", "events", "items", "questions", "reports" };
-            this.algorithm = "hmac256";
+            this.algorithm = "hmac-sha256";


changed name to hmac-sha256

bhavya-shukla-lrn · 2023-01-31T11:57:57Z

LearnositySDK/Request/Init.cs

+        private string hashValue(string prehash, string secret )
        {
-            string hash = Tools.hash(this.algorithm, prehash, value);
+            string hash = Tools.hash(this.algorithm, prehash, secret);


Added summary for params , changed params name to secret as per the comments given

bhavya-shukla-lrn · 2023-01-31T12:00:46Z

LearnositySDK/Request/Init.cs


-                        signatureList.Add(this.securityPacket.getString("user_id"));
-                        signatureList.Add(this.secret);
+


Will remove extra lines in my next commit

bhavya-shukla-lrn · 2023-01-31T12:02:34Z

LearnositySDK/Request/Init.cs

                        }

-                        signatureList.Add(this.securityPacket.getString("user_id"));
-                        signatureList.Add(this.secret);


As per the review signatureList initialisation and declaration has been removed

bhavya-shukla-lrn · 2023-01-31T12:03:22Z

LearnositySDK/Request/Init.cs

                            {
                                string user_id = users[i];
-                                hashedUsers.set(user_id, Tools.hash(this.algorithm,this.prehashString, user_id + this.secret));
+                                hashedUsers.set(user_id, Tools.hash(this.algorithm, this.prehashString, user_id + this.secret));


added space after comma

bhavya-shukla-lrn · 2023-01-31T12:04:10Z

LearnositySDK/Utils/CryptoUtil.cs

+        ///<param name="secret">String used for encryption</param>
        /// <returns>64-character hex string</returns>
-        public static string hmac256(string prehash,string value)
+        public static string hmacsha256(string prehash, string secret)


renamed algo to hmacsha256

renamed param value to secret and also added function summary with params

bhavya-shukla-lrn · 2023-01-31T12:06:17Z

LearnositySDK/Utils/Tools.cs

-        /// <param name="prehash"></param>
-        ///<param name="value"></param>
+        /// <param name="prehash">String to be hashed</param>
+        ///<param name="secret">String used for encryption</param>


added comments for params

bhavya-shukla-lrn · 2023-01-31T12:07:23Z

LearnositySDK/Utils/Tools.cs

+        /// <param name="secret">String used for encryption</param>
        /// <returns></returns>
-        public static string hash(string algorithm, string prehash, string value)
+        public static string hash(string algorithm, string prehash, string secret)


renamed value to secret(params)

bhavya-shukla-lrn · 2023-01-31T12:08:10Z

LearnositySDK/Utils/Tools.cs

            {
-                case "hmac256":
-                    str = Tools.hmac256(prehash, value);
+                case "hmac-sha256":


changed name to hmac-sha256

markus-liang-lrn · 2023-02-21T15:19:23Z

LearnosityDemo/Pages/AuthorAPIDemo.cshtml.cs

+            // item_edit mode and item_list mode
+            // the below example generate initOptions for item_edit mode
+            // more information about item_list mode can be found in initializeItemList() function
+            Init init = (mode == "item_list") ? initializeItemEdit() : initializeItemList();


is this conditional correct? if mode is item_list, then you will return itemEdit, and if mode is not item_list then you return item_list?

ohh I need to correct that , If mode is Item list it should return itemList not item edit. Thanks For pointing it out I will make that change

markus-liang-lrn · 2023-02-21T15:56:26Z

LearnosityDemo/Pages/AuthorAPIDemo.cshtml.cs

            // the below example generate initOptions for item_edit mode
            // more information about item_list mode can be found in initializeItemList() function
-            Init init = (mode == "item_list") ? initializeItemEdit() : initializeItemList();
+            Init init = (mode == "item_list") ?  initializeItemList() : initializeItemEdit();


looks good, just need to remove unnecessary space after '?'

LearnosityDemo/Pages/QuestionsAPIDemo.cshtml

LearnosityDemo/Pages/ItemsAPIDemo.cshtml

TheRealEdDawson

I left some minor comments, but generally, looks great, guys. Please go ahead!

TheRealEdDawson

Looks great. Thanks @bhavya-shukla-lrn , please go ahead with it.

LearnositySDK/LearnositySDK.csproj

TheRealEdDawson

Looks good @bhavya-shukla-lrn @markus-liang-lrn @markkellyeire, I made some minor edits to text in "README.md" and to the copyright year in "LearnositySDK.csproj", to change it to this year "2023".

…ages for the new signature LRN-38572 Change in branch

bhavya-shukla-lrn force-pushed the LRN-38974/Create-demo-page-questions-api branch 8 times, most recently from 31242ff to ba1d0ef Compare January 30, 2023 13:23

markus-liang-lrn reviewed Jan 31, 2023

View reviewed changes

bhavya-shukla-lrn commented Jan 31, 2023

View reviewed changes

bhavya-shukla-lrn force-pushed the LRN-38974/Create-demo-page-questions-api branch 2 times, most recently from 82ef665 to a82f88c Compare February 21, 2023 09:28

markus-liang-lrn reviewed Feb 21, 2023

View reviewed changes

bhavya-shukla-lrn force-pushed the LRN-38974/Create-demo-page-questions-api branch from 0e63b85 to 79fdbd6 Compare February 24, 2023 12:01

bhavya-shukla-lrn force-pushed the LRN-38974/Create-demo-page-questions-api branch 5 times, most recently from bc57763 to 083e761 Compare February 28, 2023 12:58

bhavya-shukla-lrn force-pushed the LRN-38974/Create-demo-page-questions-api branch 4 times, most recently from af9ba81 to 019801f Compare March 29, 2023 15:43

markus-liang-lrn reviewed Apr 12, 2023

View reviewed changes

LearnosityDemo/Pages/QuestionsAPIDemo.cshtml Outdated Show resolved Hide resolved

TheRealEdDawson reviewed Jun 21, 2023

View reviewed changes

LearnosityDemo/Pages/ItemsAPIDemo.cshtml Outdated Show resolved Hide resolved

TheRealEdDawson approved these changes Jun 22, 2023

View reviewed changes

TheRealEdDawson approved these changes Jun 23, 2023

View reviewed changes

bhavya-shukla-lrn force-pushed the LRN-38974/Create-demo-page-questions-api branch from d10e1e2 to 9abab5a Compare June 26, 2023 11:27

TheRealEdDawson reviewed Jun 27, 2023

View reviewed changes

LearnositySDK/LearnositySDK.csproj Outdated Show resolved Hide resolved

TheRealEdDawson approved these changes Jun 27, 2023

View reviewed changes

bhavya-shukla-lrn force-pushed the LRN-38974/Create-demo-page-questions-api branch from abe9b6b to 04f6de5 Compare June 29, 2023 06:44

[Feature] Added hmac256 signature and fixed tests also created demo p…

a6bf743

…ages for the new signature LRN-38572 Change in branch

bhavya-shukla-lrn force-pushed the LRN-38974/Create-demo-page-questions-api branch from 04f6de5 to a6bf743 Compare June 29, 2023 06:49

bhavya-shukla-lrn merged commit 5c19004 into master Jun 29, 2023

bhavya-shukla-lrn deleted the LRN-38974/Create-demo-page-questions-api branch June 29, 2023 06:50


		signatureList.Add(this.securityPacket.getString("user_id"));
		signatureList.Add(this.secret);

[feature]Adding JSON structure with reference to Question api initial… #43

[feature]Adding JSON structure with reference to Question api initial… #43

Uh oh!

Conversation

bhavya-shukla-lrn commented Jan 23, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markus-liang-lrn Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bhavya-shukla-lrn Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

TheRealEdDawson left a comment

Choose a reason for hiding this comment

Uh oh!

TheRealEdDawson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheRealEdDawson left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

markus-liang-lrn Jan 31, 2023 •

edited

Loading

bhavya-shukla-lrn Jan 31, 2023 •

edited

Loading