From 55f5dc4e0a27a7738736c33d345bcdf2fa122a11 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Raddaoui=20Mar=C3=ADn?=
 <raddaouimarin@gmail.com>
Date: Thu, 14 Nov 2024 07:51:40 +0100
Subject: [PATCH] Fix API ABAC for access tokens without attributes

Make sure we return an empty slice of attributes in the claim if no
Enduro attributes are found in the token after parsing, filtering and
mapping roles. A nil attributes slice is considered as ABAC disabled
in the API endpoints check.
---
 internal/api/auth/token_verifier.go      |  4 +-
 internal/api/auth/token_verifier_test.go | 64 ++++++++++++++++++++++++
 internal/workflow/processing.go          |  1 -
 3 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/internal/api/auth/token_verifier.go b/internal/api/auth/token_verifier.go
index 34f19b62..8d8a62bf 100644
--- a/internal/api/auth/token_verifier.go
+++ b/internal/api/auth/token_verifier.go
@@ -108,7 +108,7 @@ func (t *OIDCTokenVerifier) parseAttributes(token *oidc.IDToken) ([]string, erro
 				)
 			}
 
-			var filteredValue []string
+			filteredValue := []string{}
 			for i := range val.Len() {
 				str, ok := val.Index(i).Interface().(string)
 				if ok {
@@ -122,7 +122,7 @@ func (t *OIDCTokenVerifier) parseAttributes(token *oidc.IDToken) ([]string, erro
 				return filteredValue, nil
 			}
 
-			var attributes []string
+			attributes := []string{}
 			for _, role := range filteredValue {
 				if attrs, ok := t.cfg.ABAC.RolesMapping[role]; ok {
 					attributes = append(attributes, attrs...)
diff --git a/internal/api/auth/token_verifier_test.go b/internal/api/auth/token_verifier_test.go
index f9385ec9..8e71c906 100644
--- a/internal/api/auth/token_verifier_test.go
+++ b/internal/api/auth/token_verifier_test.go
@@ -183,6 +183,47 @@ func TestParseAttributes(t *testing.T) {
 				Attributes:    []string{"*"},
 			},
 		},
+		{
+			name: "Parses attributes based on configuration (disabled)",
+			config: &auth.OIDCConfig{
+				ProviderURL: iss,
+				ClientID:    audience,
+				ABAC: auth.OIDCABACConfig{
+					Enabled: false,
+				},
+			},
+			token: token(t, signer, iss, customClaims{
+				Email:         "info@artefactual.com",
+				EmailVerified: true,
+				Attributes:    []string{"*"},
+			}),
+			wantClaims: &auth.Claims{
+				Email:         "info@artefactual.com",
+				EmailVerified: true,
+				Attributes:    nil,
+			},
+		},
+		{
+			name: "Parses attributes based on configuration (no attributes)",
+			config: &auth.OIDCConfig{
+				ProviderURL: iss,
+				ClientID:    audience,
+				ABAC: auth.OIDCABACConfig{
+					Enabled:   true,
+					ClaimPath: "attributes",
+				},
+			},
+			token: token(t, signer, iss, customClaims{
+				Email:         "info@artefactual.com",
+				EmailVerified: true,
+				Attributes:    []string{},
+			}),
+			wantClaims: &auth.Claims{
+				Email:         "info@artefactual.com",
+				EmailVerified: true,
+				Attributes:    []string{},
+			},
+		},
 		{
 			name: "Parses attributes based on configuration (nested)",
 			config: &auth.OIDCConfig{
@@ -254,6 +295,29 @@ func TestParseAttributes(t *testing.T) {
 				Attributes:    []string{"*", "package:list", "package:listActions", "package:move", "package:read", "package:upload"},
 			},
 		},
+		{
+			name: "Parses attributes based on configuration (mapping roles, no attributes)",
+			config: &auth.OIDCConfig{
+				ProviderURL: iss,
+				ClientID:    audience,
+				ABAC: auth.OIDCABACConfig{
+					Enabled:      true,
+					ClaimPath:    "attributes",
+					UseRoles:     true,
+					RolesMapping: map[string][]string{"admin": {"*"}},
+				},
+			},
+			token: token(t, signer, iss, customClaims{
+				Email:         "info@artefactual.com",
+				EmailVerified: true,
+				Attributes:    []string{"other", "random", "role"},
+			}),
+			wantClaims: &auth.Claims{
+				Email:         "info@artefactual.com",
+				EmailVerified: true,
+				Attributes:    []string{},
+			},
+		},
 		{
 			name: "Fails to parse attributes (missing claim)",
 			config: &auth.OIDCConfig{
diff --git a/internal/workflow/processing.go b/internal/workflow/processing.go
index 035e273e..fb16e68e 100644
--- a/internal/workflow/processing.go
+++ b/internal/workflow/processing.go
@@ -1284,7 +1284,6 @@ func (w *ProcessingWorkflow) validatePREMIS(
 		XMLPath: xmlPath,
 		XSDPath: w.cfg.ValidatePREMIS.XSDPath,
 	}).Get(activityOpts, &xmlvalidateResult)
-
 	if err != nil {
 		if strings.Contains(err.Error(), fmt.Sprintf("%s: no such file or directory", xmlPath)) {
 			// Allow PREMIS XML to not exist without failing workflow.