Fix tests

nexus-main · Mar 7, 2024 · 6ec808c · 6ec808c
1 parent 9c86b77
commit 6ec808c
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 42 deletions.
diff --git a/notes/auth.md b/notes/auth.md
@@ -1,7 +1,13 @@
 # Note
-The text below does not apply anymore to Nexus because we have switched from refresh tokens + access tokens to personal access tokens that expire only optionally and are not cryptographically signed but checked against the database instead. The negible problem of higher database load is acceptible to get the benefit of not having to manage refresh tokens which are prone to being revoked as soon as the user uses it in more than a single place.
+The text below does not fully apply anymore to Nexus because we have switched from refresh tokens + access tokens to personal access tokens that expire only optionally and are not cryptographically signed but checked against the database instead. The negible problem of higher database load is acceptible to get the benefit of not having to manage refresh tokens which are prone to being revoked as soon as the user uses it in more than a single place.
 
-The new personal access tokens approach allows fine-grained access control to catalogs and makes many parts of the code much simpler.
+The new personal access tokens approach allows fine-grained access control to catalogs and makes many parts of the code much simpler. Current status is:
+- User can manage personal access tokens in the web interface and specify read or read/write access to specific catalogs.
+- The token the user gets is a string which consists of a combination of the token secret (a long random base64 encoded number) and the user id.
+- Tokens are stored on disk in the folder configured by the `PathsOptions.Users` option in a files named `tokens.json`. They loaded lazily into memory on first demand and kept there for future requests.
+- When the token is part of the Authorization header (`Authorization: Bearer <token>`) it is being handles by the `PersonalAccessTokenAuthenticationHandler` which creates a `ClaimsPrincipal` if the token is valid.
+- The claims that are associated with the token can be anything but right now only the claims `CanReadCatalog` and `CanWriteCatalog` are being considered. To avoid a token to be more powerful than the user itself, the user claims are also being checked (see `AuthUtilities.cs`) on each request.
+- The lifetime of the tokens can be choosen by the users or left untouched to produce tokens with unlimited lifetime.
 
 # Authentication and Authorization
 
@@ -55,7 +61,7 @@ In order to detect a compromised token, it is recommended to implement token rot
 
 ## Implementation details
 
-The backend of Nexus is a confidential client upon user request, it will perform the authorization code flow to obtain an ID token to authenticate and sign-in the user.
+The backend of Nexus is a confidential client and upon user request, it will perform the authorization code flow to obtain an ID token to authenticate and sign-in the user.
 
 Nexus supports multiple OpenID Connect providers. See [Configuration] on how to add configuration values.
 
@@ -71,7 +77,7 @@ The problem now is that although the access token contains the subject claim, it
 
 Another problem is that Nexus cannot add these user-specific claims to the access token, which means that the user database must be consulted for every single request, resulting in a high disk load.
 
-Also, a such client would be public which means it is possible to copy the `client_id` and use them in other clients, which might be problematic when there is limited traffic allowed .
+Also, a such client would be public which means it is possible to copy the `client_id` and use them in other clients, which might be problematic when there is limited traffic allowed.
 
 The last problem with refresh tokens is that _"for public clients [they] MUST be sender-constrained or use
    refresh token rotation [...]"_ [[OAuth 2.0 Security Best Current Practice](https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-19#section-2.2.2), [RFC 6749](https://datatracker.ietf.org/doc/html/rfc6749#section-4.13)].

diff --git a/src/Nexus/Utilities/AuthUtilities.cs b/src/Nexus/Utilities/AuthUtilities.cs
@@ -31,8 +31,8 @@ public static bool IsCatalogReadable(
                 catalogMetadata,
                 owner,
                 user,
-                singleClaimValue: NexusClaims.CAN_READ_CATALOG,
-                groupClaimValue: NexusClaims.CAN_READ_CATALOG,
+                singleClaimType: NexusClaims.CAN_READ_CATALOG,
+                groupClaimType: NexusClaims.CAN_READ_CATALOG_GROUP,
                 checkImplicitAccess: true
             );
         }
@@ -47,8 +47,8 @@ public static bool IsCatalogWritable(
                 catalogMetadata,
                 owner: default,
                 user,
-                singleClaimValue: NexusClaims.CAN_READ_CATALOG,
-                groupClaimValue: NexusClaims.CAN_WRITE_CATALOG,
+                singleClaimType: NexusClaims.CAN_WRITE_CATALOG,
+                groupClaimType: NexusClaims.CAN_WRITE_CATALOG_GROUP,
                 checkImplicitAccess: false
             );
         }
@@ -58,8 +58,8 @@ private static bool InternalIsCatalogAccessible(
             CatalogMetadata catalogMetadata, 
             ClaimsPrincipal? owner, 
             ClaimsPrincipal user,
-            string singleClaimValue,
-            string groupClaimValue,
+            string singleClaimType,
+            string groupClaimType,
             bool checkImplicitAccess)
         {
             foreach (var identity in user.Identities)
@@ -92,7 +92,7 @@ private static bool InternalIsCatalogAccessible(
                     /* The token alone can access the catalog ... */
                     var canAccessCatalog = identity.HasClaim(
                         claim => 
-                            claim.Type == singleClaimValue &&
+                            claim.Type == singleClaimType &&
                             Regex.IsMatch(catalogId, claim.Value)
                     );
 
@@ -106,8 +106,8 @@ private static bool InternalIsCatalogAccessible(
                             catalogMetadata, 
                             owner, 
                             identity,
-                            NexusClaims.ToPatUserClaimType(singleClaimValue), 
-                            NexusClaims.ToPatUserClaimType(groupClaimValue));
+                            NexusClaims.ToPatUserClaimType(singleClaimType), 
+                            NexusClaims.ToPatUserClaimType(groupClaimType));
                     }
                 }
 
@@ -127,8 +127,8 @@ private static bool InternalIsCatalogAccessible(
                         catalogMetadata, 
                         owner, 
                         identity, 
-                        singleClaimValue, 
-                        groupClaimValue);
+                        singleClaimType, 
+                        groupClaimType);
                 }
 
                 /* leave loop when access is granted */
@@ -144,8 +144,8 @@ private static bool CanUserAccessCatalog(
             CatalogMetadata catalogMetadata, 
             ClaimsPrincipal? owner, 
             ClaimsIdentity identity,
-            string singleClaimValue,
-            string groupClaimValue
+            string singleClaimType,
+            string groupClaimType
         )
         {
             var isOwner = 
@@ -154,13 +154,13 @@ owner is not null &&
 
             var canReadCatalog = identity.HasClaim(
                 claim => 
-                    claim.Type == singleClaimValue &&
+                    claim.Type == singleClaimType &&
                     Regex.IsMatch(catalogId, claim.Value)
             );
 
             var canReadCatalogGroup = catalogMetadata.GroupMemberships is not null && identity.HasClaim(
                 claim => 
-                    claim.Type == groupClaimValue &&
+                    claim.Type == groupClaimType &&
                     catalogMetadata.GroupMemberships.Any(group => Regex.IsMatch(group, claim.Value))
             );
 

diff --git a/tests/Nexus.Tests/Other/UtilitiesTests.cs b/tests/Nexus.Tests/Other/UtilitiesTests.cs
@@ -12,35 +12,47 @@ public class UtilitiesTests
     {
         [Theory]
 
-        [InlineData("Basic", true, new string[0], new string[0], true)]
-        [InlineData("Basic", false, new string[] { "/D/E/F", "/A/B/C", "/G/H/I" }, new string[0], true)]
-        [InlineData("Basic", false, new string[] { "^/A/B/.*" }, new string[0], true)]
-        [InlineData("Basic", false, new string[0], new string[] { "A" }, true)]
-
-        [InlineData("Basic", false, new string[0], new string[0], false)]
-        [InlineData("Basic", false, new string[] { "/D/E/F", "/A/B/C2", "/G/H/I" }, new string[0], false)]
-        [InlineData("Basic", false, new string[0], new string[] { "A2" }, false)]
-        [InlineData(null, true, new string[0], new string[0], false)]
-        public void CanDetermineCatalogAccessibility(
+        [InlineData("Basic", true, new string[0], new string[0], new string[0], true)]
+        [InlineData("Basic", false, new string[] { "/D/E/F", "/A/B/C", "/G/H/I" }, new string[0], new string[0], true)]
+        [InlineData("Basic", false, new string[] { "^/A/B/.*" }, new string[0], new string[0], true)]
+        [InlineData("Basic", false, new string[0], new string[] { "A" }, new string[0], true)]
+
+        [InlineData("Basic", false, new string[0], new string[0], new string[0], false)]
+        [InlineData("Basic", false, new string[] { "/D/E/F", "/A/B/C2", "/G/H/I" }, new string[0], new string[0], false)]
+        [InlineData("Basic", false, new string[0], new string[] { "A2" }, new string[0], false)]
+        [InlineData(null, true, new string[0], new string[0], new string[0], false)]
+
+        [InlineData(PersonalAccessTokenAuthenticationDefaults.AuthenticationScheme, true, new string[0], new string[0], new string[0], true)]
+        [InlineData(PersonalAccessTokenAuthenticationDefaults.AuthenticationScheme, false, new string[] { "/A/B/C" }, new string[0], new string[] { "/A/B/" }, true)]
+        [InlineData(PersonalAccessTokenAuthenticationDefaults.AuthenticationScheme, false, new string[] { "/A/B/C" }, new string[0], new string[] { "/D/E/" }, false)]
+        public void CanDetermineCatalogReadability(
             string? authenticationType,
             bool isAdmin,
             string[] canReadCatalog,
             string[] canReadCatalogGroup,
+            string[] patUserCanReadCatalog,
             bool expected)
         {
             // Arrange
+            var isPAT = authenticationType == PersonalAccessTokenAuthenticationDefaults.AuthenticationScheme;
+
+            var roleClaimType = isPAT
+                ? NexusClaims.ToPatUserClaimType(Claims.Role)
+                : Claims.Role;
+
             var catalogId = "/A/B/C";
-            var catalogMetadata = new CatalogMetadata(default, GroupMemberships: new[] { "A" }, default);
+            var catalogMetadata = new CatalogMetadata(default, GroupMemberships: ["A"], default);
 
             var adminClaim = isAdmin
-                ? new Claim[] { new Claim(Claims.Role, NexusRoles.ADMINISTRATOR) }
+                ? [new Claim(roleClaimType, NexusRoles.ADMINISTRATOR)]
                 : Array.Empty<Claim>();
 
             var principal = new ClaimsPrincipal(
                 new ClaimsIdentity(
                     claims: adminClaim
                         .Concat(canReadCatalog.Select(value => new Claim(NexusClaims.CAN_READ_CATALOG, value)))
-                        .Concat(canReadCatalogGroup.Select(value => new Claim(NexusClaims.CAN_READ_CATALOG_GROUP, value))),
+                        .Concat(canReadCatalogGroup.Select(value => new Claim(NexusClaims.CAN_READ_CATALOG_GROUP, value)))
+                        .Concat(patUserCanReadCatalog.Select(value => new Claim(NexusClaims.ToPatUserClaimType(NexusClaims.CAN_READ_CATALOG), value))),
                     authenticationType,
                     nameType: Claims.Name,
                     roleType: Claims.Role));
@@ -52,33 +64,49 @@ public void CanDetermineCatalogAccessibility(
             Assert.Equal(expected, actual);
         }
 
-        // TODO Extend test for 'CAN_WRITE_CATALOG'
         [Theory]
 
-        [InlineData("Basic", true, new string[0], true)]
-        [InlineData("Basic", false, new string[] { "/D/E/F", "/A/B/C", "/G/H/I" }, true)]
-        [InlineData("Basic", false, new string[] { "^/A/B/.*" }, true)]
+        [InlineData("Basic", true, new string[0], new string[0], new string[0], true)]
+        [InlineData("Basic", false, new string[] { "/D/E/F", "/A/B/C", "/G/H/I" }, new string[0], new string[0], true)]
+        [InlineData("Basic", false, new string[] { "^/A/B/.*" }, new string[0], new string[0], true)]
+        [InlineData("Basic", false, new string[0], new string[] { "A" }, new string[0], true)]
+
+        [InlineData("Basic", false, new string[0], new string[0], new string[0], false)]
+        [InlineData("Basic", false, new string[] { "/D/E/F", "/A/B/C2", "/G/H/I" }, new string[0], new string[0], false)]
+        [InlineData("Basic", false, new string[0], new string[] { "A2" }, new string[0], false)]
+        [InlineData(null, true, new string[0], new string[0], new string[0], false)]
 
-        [InlineData("Basic", false, new string[0], false)]
-        [InlineData(null, true, new string[0], false)]
-        public void CanDetermineCatalogEditability(
+        [InlineData(PersonalAccessTokenAuthenticationDefaults.AuthenticationScheme, true, new string[0], new string[0], new string[0], true)]
+        [InlineData(PersonalAccessTokenAuthenticationDefaults.AuthenticationScheme, false, new string[] { "/A/B/C" }, new string[0], new string[] { "/A/B/" }, true)]
+        [InlineData(PersonalAccessTokenAuthenticationDefaults.AuthenticationScheme, false, new string[] { "/A/B/C" }, new string[0], new string[] { "/D/E/" }, false)]
+        public void CanDetermineCatalogWritability(
             string? authenticationType,
             bool isAdmin,
             string[] canWriteCatalog,
+            string[] canWriteCatalogGroup,
+            string[] patUserCanWriteCatalog,
             bool expected)
         {
             // Arrange
+            var isPAT = authenticationType == PersonalAccessTokenAuthenticationDefaults.AuthenticationScheme;
+
+            var roleClaimType = isPAT
+                ? NexusClaims.ToPatUserClaimType(Claims.Role)
+                : Claims.Role;
+
             var catalogId = "/A/B/C";
-            var catalogMetadata = new CatalogMetadata(default, GroupMemberships: new[] { "A" }, default);
+            var catalogMetadata = new CatalogMetadata(default, GroupMemberships: ["A"], default);
 
             var adminClaim = isAdmin
-                ? new Claim[] { new Claim(Claims.Role, NexusRoles.ADMINISTRATOR) }
+                ? [new Claim(roleClaimType, NexusRoles.ADMINISTRATOR)]
                 : Array.Empty<Claim>();
 
             var principal = new ClaimsPrincipal(
                 new ClaimsIdentity(
                     claims: adminClaim
-                        .Concat(canWriteCatalog.Select(value => new Claim(NexusClaims.CAN_WRITE_CATALOG, value))),
+                        .Concat(canWriteCatalog.Select(value => new Claim(NexusClaims.CAN_WRITE_CATALOG, value)))
+                        .Concat(canWriteCatalogGroup.Select(value => new Claim(NexusClaims.CAN_WRITE_CATALOG_GROUP, value)))
+                        .Concat(patUserCanWriteCatalog.Select(value => new Claim(NexusClaims.ToPatUserClaimType(NexusClaims.CAN_WRITE_CATALOG), value))),
                     authenticationType,
                     nameType: Claims.Name,
                     roleType: Claims.Role));