Skip to content

Commit

Permalink
Check illegal characters (#16)
Browse files Browse the repository at this point in the history
* Add test - not passing

* Add comments and testdata dir

* Make the tests pass.

* Add more tests

* Check illegal characters

* Fix the tests

* Add test for checkMetadata function (#4)

* Add test - not passing

* Add comments and testdata dir

* Make the tests pass.

* Add more tests

* Mark a line for future improvement

* Bump golang.org/x/crypto from 0.3.0 to 0.17.0 (#19)

Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.3.0 to 0.17.0.
- [Commits](golang/crypto@v0.3.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add dummy email

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
  • Loading branch information
kavir1698 and dependabot[bot] authored Mar 27, 2024
1 parent b961667 commit 2f74270
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 11 deletions.
51 changes: 48 additions & 3 deletions datasetIngestor/checkMetadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package datasetIngestor

import (
"bytes"
"github.com/paulscherrerinstitute/scicat/datasetUtils"
"encoding/json"
"github.com/paulscherrerinstitute/scicat/datasetUtils"
"io/ioutil"
"log"
"net"
Expand Down Expand Up @@ -68,13 +68,18 @@ func CheckMetadata(client *http.Client, APIServer string, metadatafile string, u
}

// Use type assertion to convert the interface{} object to a map[string]interface{}.
metaDataMap = metadataObj.(map[string]interface{}) // `.(` is type assertion: a way to extract the underlying value of an interface and check whether it's of a specific type.
metaDataMap = metadataObj.(map[string]interface{}) // `.(` is type assertion: a way to extract the underlying value of an interface and check whether it's of a specific type.
beamlineAccount = false

// Check scientificMetadata for illegal keys
if checkIllegalKeys(metaDataMap) {
panic("Metadata contains keys with illegal characters (., [], $, or <>).")
}

// If the user is not the ingestor, check whether any of the accessGroups equal the ownerGroup. Otherwise, check for beamline-specific accounts.
if user["displayName"] != "ingestor" {
// Check if the metadata contains the "ownerGroup" key.
if ownerGroup, ok := metaDataMap["ownerGroup"]; ok { // type assertion with a comma-ok idiom
if ownerGroup, ok := metaDataMap["ownerGroup"]; ok { // type assertion with a comma-ok idiom
validOwner := false
// Iterate over accessGroups to validate the owner group.
for _, b := range accessGroups {
Expand Down Expand Up @@ -226,6 +231,9 @@ func CheckMetadata(client *http.Client, APIServer string, metadatafile string, u
//fmt.Printf("Marshalled meta data : %s\n", string(bmm))
// now check validity
req, err := http.NewRequest("POST", myurl, bytes.NewBuffer(bmm))
if err != nil {
log.Fatal(err)
}
req.Header.Set("Content-Type", "application/json")

resp, err := client.Do(req)
Expand Down Expand Up @@ -271,3 +279,40 @@ func CheckMetadata(client *http.Client, APIServer string, metadatafile string, u
return metaDataMap, sourceFolder, beamlineAccount

}

func checkIllegalKeys(metadata map[string]interface{}) bool {
for key, value := range metadata {
if containsIllegalCharacters(key) {
return true
}

switch v := value.(type) {
case map[string]interface{}:
if checkIllegalKeys(v) {
return true
}
case []interface{}:
for _, item := range v {
switch itemValue := item.(type) { // Type switch on array item
case map[string]interface{}:
if checkIllegalKeys(itemValue) {
return true
}
// Add other cases if needed
}
}
}
}
return false
}

func containsIllegalCharacters(s string) bool {
// Check if the string contains periods, brackets, or other illegal characters
// You can adjust this condition based on your specific requirements
for _, char := range s {
if char == '.' || char == '[' || char == ']' || char == '<' || char == '>' || char == '$' {
return true
}
}
return false
}
44 changes: 43 additions & 1 deletion datasetIngestor/checkMetadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ func TestCheckMetadata(t *testing.T) {
}

// Mock access groups
accessGroups := []string{"p17880", "p17301"}
accessGroups := []string{"group1", "group2"}


// Call the function with mock parameters
metaDataMap, sourceFolder, beamlineAccount := CheckMetadata(client, APIServer, metadatafile1, user, accessGroups)
Expand Down Expand Up @@ -116,3 +117,44 @@ func TestCheckMetadata(t *testing.T) {
t.Error("Expected beamlineAccount to be false")
}
}

func TestCheckMetadata_CrashCase(t *testing.T) {
defer func() {
if recover() != nil {
t.Log("Function crashed as expected")
} else {
t.Fatal("Function did not crash as expected")
}
}()

// Define mock parameters for the function
var TEST_API_SERVER string = "https://dacat-qa.psi.ch/api/v3"
var APIServer = TEST_API_SERVER
var metadatafile3 = "testdata/metadata_illegal.json"

// Mock HTTP client
client := &http.Client{
Timeout: 5 * time.Second, // Set a timeout for requests
Transport: &http.Transport{
// Customize the transport settings if needed (e.g., proxy, TLS config)
// For a dummy client, default settings are usually sufficient
},
CheckRedirect: func(req *http.Request, via []*http.Request) error {
// Customize how redirects are handled if needed
// For a dummy client, default behavior is usually sufficient
return http.ErrUseLastResponse // Use the last response for redirects
},
}

// Mock user map
user := map[string]string{
"displayName": "csaxsswissfel",
"mail": "testuser@example.com",
}

// Mock access groups
accessGroups := []string{"group1", "group2"}
// Call the function that should crash
CheckMetadata(client, APIServer, metadatafile3, user, accessGroups)
}

7 changes: 4 additions & 3 deletions datasetIngestor/testdata/metadata-short.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
{
"principalInvestigator":"egon.meier@psi.ch",
"principalInvestigator":"piemail@example.ch",
"creationLocation":"/PSI/SLS/CSAXS/SWISSFEL",
"sourceFolder": "/tmp/gnome",
"owner": "Andreas Menzel",
"owner": "first last",
"type": "raw",
"ownerGroup": "p17880"
"ownerGroup": "group1"

}
9 changes: 5 additions & 4 deletions datasetIngestor/testdata/metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
"creationLocation": "/PSI/SLS/CSAXS/SWISSFEL",
"datasetName": "CMakeCache",
"description": "",
"owner": "Ana Diaz",
"ownerEmail": "ana.diaz@psi.ch",
"ownerGroup": "p17301",
"principalInvestigator": "ana.diaz@psi.ch",
"owner": "first last",
"ownerEmail": "test@example.com",
"ownerGroup": "group1",
"principalInvestigator": "test@example.com",

"scientificMetadata": [
{
"sample": {
Expand Down
20 changes: 20 additions & 0 deletions datasetIngestor/testdata/metadata_illegal.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"creationLocation": "/PSI/SLS/CSAXS",
"datasetName": "CMakeCache",
"description": "",
"owner": "first last",
"ownerEmail": "test@example.com",
"ownerGroup": "group1",
"principalInvestigator": "test@example.com",
"scientificMetadata": [
{
"sample": {
"description.": "It has an illegal characters in the key",
"name]": "same",
"principalInvestigator": ""
}
}
],
"sourceFolder": "/usr/share/gnome",
"type": "raw"
}

0 comments on commit 2f74270

Please sign in to comment.