Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secretserver develop #1092

Merged

Conversation

Deepshikha8514
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Aug 29, 2022

Codecov Report

Base: 64.57% // Head: 64.55% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (0bc5cd2) compared to base (9a6037e).
Patch coverage: 2.85% of modified lines in pull request are covered.

❗ Current head 0bc5cd2 differs from pull request most recent head d8ce667. Consider uploading reports for the commit d8ce667 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1092      +/-   ##
===========================================
- Coverage    64.57%   64.55%   -0.03%     
===========================================
  Files          541      541              
  Lines        50910    50926      +16     
===========================================
  Hits         32877    32877              
- Misses       18033    18049      +16     
Impacted Files Coverage Δ
...dules/secretserver/stix_transmission/api_client.py 9.12% <0.00%> (-0.60%) ⬇️
...tix_translation/test_secretserver_stix_to_query.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -14,28 +14,21 @@
}
},

"user-account": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this and stick it in a custom object? When possible, standard STIX objects should be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done the changes

"x-secret": {
"fields": {
"secret_name": ["SecretName"],
"user_name": ["username"],
"username": ["EmailAddress"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is an email address, it should go under the email-addr:value property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"x-secret": {
"fields": {
"secret_name": ["SecretName"],
"user_name": ["username"],
"username": ["Unique_Identtification"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this go in user-account:display_name instead of this custom object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have done our db parser changes with the above mapping. If it is not mandatory, Can it be taken for next release?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is Unique_Identification different from the username field? This can likely remain where it is if they are different.

"secret_id": ["ItemId"],
"user_id": ["UserId"]
"user_id": ["UserId"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not just in the user-account:user_id object? No need to put it in a custom object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"x-secret": {
"fields": {
"secret_name": ["SecretName"],
"username": ["EmailAddress"],
Copy link
Collaborator

@delliott90 delliott90 Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in a email-addr:value property and not a custom object

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"secret_name": ["SecretName"],
"username": ["EmailAddress"],
"secret_id": ["ItemId"],
"user_id": ["UserId"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in a user-account:user_id property

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"username": ["EmailAddress"],
"secret_id": ["ItemId"],
"user_id": ["UserId"],
"server_user_name" : ["Username"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in a user-account:display_name property

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have done our db parser changes with the above mapping. If it is not mandatory, Can it be taken for next release?

"user_id": [
"UserId"
]
"user_name": ["username"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property needs to be display_name, there is no user_name property on the user-account object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done the changes

{
"username": [
{
"key": "user-account.user_name"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be user-account.display_name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done the changes

@delliott90 delliott90 merged commit ceeb9bb into opencybersecurityalliance:develop Sep 26, 2022
delliott90 pushed a commit that referenced this pull request Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants