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

Added Controls class as an Element instance variable, moved control b… #177

Merged
merged 4 commits into from Oct 31, 2021

Conversation

nozmore
Copy link
Collaborator

@nozmore nozmore commented Sep 29, 2021

…ased annotations to control class, updated threatlib, enabled json output and updated existing tests.

…ased annotations to control class, updated threatlib, enabled json output and updated existing tests.
@nozmore nozmore requested a review from izar October 13, 2021 02:03
@nozmore
Copy link
Collaborator Author

nozmore commented Oct 13, 2021

I still need some comments on this one. Its still draft but is functional, I think the only thing left is to remove the commented out control annotations on the various Element subclasses.

I had previously started some discussion on Slack but didn't get a thumbs up or down.

I wanted to have some separation between controls. I think there are still room for documentation so this could allow us to focus on documenting element-based annotations ("what the thing is") then focus on the controls. Having them here we can document them once rather than some annotations are on multiple Element subclasses.

I was debating if:

  1. we should have a single Controls class or have multiple subclasses.
    I think the single class is good enough for now and we could easily have a follow on PR to separate them out to lock things down or bring clarity. I could see differences between Dataflow vs Server/Process/Lambda. The only place I would see different controls is for a future PR I plan to submit for Data. I want to define Data once then have a DataUsage object on an Element. As an example one Data control would be isValidated instead of validatesInput on an Element. With this change I want to be able to produce a diagram to see how a specific piece of data flows thru the system and where controls are in place, possibly have ties to Classification requirements.

  2. are all the attributes pulled out actually meant to be controls, there were some I didn't know what the original intent was and they were not used in the threatlib.

  3. likewise is there any controls I missed.

@ghost
Copy link

ghost commented Oct 30, 2021

CodeSee Review Map:

Review these changes using an interactive CodeSee Map

Review in an interactive map

View more CodeSee Maps

Legend

CodeSee Map Legend

@nozmore nozmore marked this pull request as ready for review October 30, 2021 04:23
@izar izar merged commit 755318b into master Oct 31, 2021
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