Merge #550

550: fix: invalid API key handling r=brunoocasali a=ahmednfwela # Pull Request ## Related issue Fixes #363 Supersedes #549 ## What does this PR do? - improve error message of `MeilisearchTenantTokenApiKeyInvalid` - improve constructor of `TenantTokenRules` to take `IReadOnlyDictionary` instead of a `Dictionary` - Added tests in `TenantTokenTests`. ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! Co-authored-by: ahmednfwela <[email protected]>
meilisearch · Jul 10, 2024 · f97dd41 · f97dd41
2 parents aadf72b + 39ca582
commit f97dd41
Show file tree

Hide file tree

Showing 5 changed files with 116 additions and 29 deletions.
diff --git a/meilisearch-dart b/meilisearch-dart
diff --git a/src/Meilisearch/Errors/MeilisearchTenantTokenApiKeyInvalid.cs b/src/Meilisearch/Errors/MeilisearchTenantTokenApiKeyInvalid.cs
@@ -12,7 +12,7 @@ public class MeilisearchTenantTokenApiKeyInvalid : Exception
         /// Initializes a new instance of the <see cref="MeilisearchTenantTokenApiKeyInvalid"/> class.
         /// </summary>
         public MeilisearchTenantTokenApiKeyInvalid()
-            : base("Cannot generate a signed token without a valid apiKey. Provide one in the MeilisearchClient instance or in the method params.")
+            : base("Cannot generate a signed token without a valid apiKey. Provide one in the MeilisearchClient instance or in the method params. The key MUST be at least 16 characters, or 128 bits")
         {
         }
     }

diff --git a/src/Meilisearch/TenantToken.cs b/src/Meilisearch/TenantToken.cs
@@ -16,12 +16,12 @@ public class TenantToken
         /// <returns>JWT string</returns>
         public static string GenerateToken(string apiKeyUid, TenantTokenRules searchRules, string apiKey, DateTime? expiresAt)
         {
-            if (String.IsNullOrEmpty(apiKeyUid))
+            if (string.IsNullOrEmpty(apiKeyUid))
             {
                 throw new MeilisearchTenantTokenApiKeyUidInvalid();
             }
 
-            if (String.IsNullOrEmpty(apiKey) || apiKey.Length < 8)
+            if (string.IsNullOrEmpty(apiKey) || apiKey.Length < 16)
             {
                 throw new MeilisearchTenantTokenApiKeyInvalid();
             }

diff --git a/src/Meilisearch/TenantTokenRules.cs b/src/Meilisearch/TenantTokenRules.cs
@@ -10,11 +10,31 @@ public class TenantTokenRules
     {
         private readonly object _rules;
 
-        public TenantTokenRules(Dictionary<string, object> rules)
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TenantTokenRules"/> class based on a rules json object.        
+        /// </summary>
+        /// <param name="rules">
+        /// 
+        /// example:
+        /// 
+        /// {'*': {"filter": 'tag = Tale'}}
+        /// 
+        /// </param>
+        public TenantTokenRules(IReadOnlyDictionary<string, object> rules)
         {
             _rules = rules;
         }
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TenantTokenRules"/> class based on a rules string array.        
+        /// </summary>
+        /// <param name="rules">
+        /// 
+        /// example:
+        /// 
+        /// ['books']
+        /// 
+        /// </param>
         public TenantTokenRules(string[] rules)
         {
             _rules = rules;

diff --git a/tests/Meilisearch.Tests/TenantTokenTests.cs b/tests/Meilisearch.Tests/TenantTokenTests.cs
@@ -7,7 +7,8 @@
 
 namespace Meilisearch.Tests
 {
-    public abstract class TenantTokenTests<TFixture> : IAsyncLifetime where TFixture : IndexFixture
+    public abstract class TenantTokenTests<TFixture> : IAsyncLifetime
+        where TFixture : IndexFixture
     {
         private readonly TenantTokenRules _searchRules = new TenantTokenRules(new string[] { "*" });
 
@@ -70,9 +71,21 @@ public void ClientThrowsIfNoKeyIsAvailable()
             );
         }
 
+
+        [Fact]
+        public void ClientThrowsIfKeyIsLessThan128Bits()
+        {
+            var customClient = new MeilisearchClient(_fixture.MeilisearchAddress(), "masterKey");
+            Assert.Throws<MeilisearchTenantTokenApiKeyInvalid>(
+                () => customClient.GenerateTenantToken(_uid, _searchRules)
+            );
+        }
+
+
+
         [Theory]
         [MemberData(nameof(PossibleSearchRules))]
-        public async void SearchesSuccessfullyWithTheNewToken(dynamic data)
+        public async void SearchesSuccessfullyWithTheNewToken(object data)
         {
             var keyOptions = new Key
             {
@@ -83,10 +96,26 @@ public async void SearchesSuccessfullyWithTheNewToken(dynamic data)
             };
             var createdKey = await _client.CreateKeyAsync(keyOptions);
             var admClient = new MeilisearchClient(_fixture.MeilisearchAddress(), createdKey.KeyUid);
-            var task = await admClient.Index(_indexName).UpdateFilterableAttributesAsync(new string[] { "tag", "book_id" });
+            var task = await admClient
+                .Index(_indexName)
+                .UpdateFilterableAttributesAsync(new string[] { "tag", "book_id" });
             await admClient.Index(_indexName).WaitForTaskAsync(task.TaskUid);
 
-            var token = admClient.GenerateTenantToken(createdKey.Uid, new TenantTokenRules(data));
+            TenantTokenRules tokenRules;
+            if (data is string[] dataStringArray)
+            {
+                tokenRules = new TenantTokenRules(dataStringArray);
+            }
+            else if (data is IReadOnlyDictionary<string, object> dataDictionary)
+            {
+                tokenRules = new TenantTokenRules(dataDictionary);
+            }
+            else
+            {
+                throw new Exception("Invalid data type");
+            }
+
+            var token = admClient.GenerateTenantToken(createdKey.Uid, tokenRules);
             var customClient = new MeilisearchClient(_fixture.MeilisearchAddress(), token);
 
             await customClient.Index(_indexName).SearchAsync<Movie>(string.Empty);
@@ -105,12 +134,17 @@ public async Task SearchFailsWhenTokenIsExpired()
             var createdKey = await _client.CreateKeyAsync(keyOptions);
             var admClient = new MeilisearchClient(_fixture.MeilisearchAddress(), createdKey.KeyUid);
 
-            var token = admClient.GenerateTenantToken(createdKey.Uid, new TenantTokenRules(new[] { "*" }), expiresAt: DateTime.UtcNow.AddSeconds(1));
+            var token = admClient.GenerateTenantToken(
+                createdKey.Uid,
+                new TenantTokenRules(new[] { "*" }),
+                expiresAt: DateTime.UtcNow.AddSeconds(1)
+            );
             var customClient = new MeilisearchClient(_fixture.MeilisearchAddress(), token);
             Thread.Sleep(TimeSpan.FromSeconds(2));
 
-            await Assert.ThrowsAsync<MeilisearchApiError>(async () =>
-                await customClient.Index(_indexName).SearchAsync<Movie>(string.Empty));
+            await Assert.ThrowsAsync<MeilisearchApiError>(
+                async () => await customClient.Index(_indexName).SearchAsync<Movie>(string.Empty)
+            );
         }
 
         [Fact]
@@ -126,29 +160,61 @@ public async void SearchSucceedsWhenTokenIsNotExpired()
             var createdKey = await _client.CreateKeyAsync(keyOptions);
             var admClient = new MeilisearchClient(_fixture.MeilisearchAddress(), createdKey.KeyUid);
 
-            var token = admClient.GenerateTenantToken(createdKey.Uid, new TenantTokenRules(new[] { "*" }), expiresAt: DateTime.UtcNow.AddMinutes(1));
+            var token = admClient.GenerateTenantToken(
+                createdKey.Uid,
+                new TenantTokenRules(new[] { "*" }),
+                expiresAt: DateTime.UtcNow.AddMinutes(1)
+            );
             var customClient = new MeilisearchClient(_fixture.MeilisearchAddress(), token);
             await customClient.Index(_indexName).SearchAsync<Movie>(string.Empty);
         }
 
-        public static IEnumerable<object[]> PossibleSearchRules()
+        public static TheoryData<object> PossibleSearchRules()
         {
-            // {'*': {}}
-            yield return new object[] { new Dictionary<string, object> { { "*", new Dictionary<string, object> { } } } };
-            // {'books': {}}
-            yield return new object[] { new Dictionary<string, object> { { "books", new Dictionary<string, object> { } } } };
-            // {'*': null}
-            yield return new object[] { new Dictionary<string, object> { { "*", null } } };
-            // {'books': null}
-            yield return new object[] { new Dictionary<string, object> { { "books", null } } };
-            // ['*']
-            yield return new object[] { new string[] { "*" } };
-            // ['books']
-            yield return new object[] { new string[] { "books" } };
-            // {'*': {"filter": 'tag = Tale'}}
-            yield return new object[] { new Dictionary<string, object> { { "*", new Dictionary<string, object> { { "filter", "tag = Tale" } } } } };
-            // {'books': {"filter": 'tag = Tale'}}
-            yield return new object[] { new Dictionary<string, object> { { "books", new Dictionary<string, object> { { "filter", "tag = Tale" } } } } };
+            IEnumerable<object> SubPossibleSearchRules()
+            {
+                // {'*': {}}
+                yield return new Dictionary<string, object>
+                {
+                    {
+                        "*",
+                        new Dictionary<string, object> { }
+                    }
+                };
+                // {'books': {}}
+                yield return new Dictionary<string, object>
+                {
+                    {
+                        "books",
+                        new Dictionary<string, object> { }
+                    }
+                };
+                // {'*': null}
+                yield return new Dictionary<string, object> { { "*", null } };
+                // {'books': null}
+                yield return new Dictionary<string, object> { { "books", null } };
+                // ['*']
+                yield return new string[] { "*" };
+                // ['books']
+                yield return new string[] { "books" };
+                // {'*': {"filter": 'tag = Tale'}}
+                yield return new Dictionary<string, object>
+                {
+                    {
+                        "*",
+                        new Dictionary<string, object> { { "filter", "tag = Tale" } }
+                    }
+                };
+                // {'books': {"filter": 'tag = Tale'}}
+                yield return new Dictionary<string, object>
+                {
+                    {
+                        "books",
+                        new Dictionary<string, object> { { "filter", "tag = Tale" } }
+                    }
+                };
+            }
+            return new TheoryData<object>(SubPossibleSearchRules());
         }
     }
 }