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

Add Zone to managed entity object #1181

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

max-power15
Copy link
Contributor

@max-power15 max-power15 commented Sep 25, 2024

Related Issue:

Managed Entity object was did not stand out as an obvious choice when reviewing Okta Network zones.

Description of changes:

  • Following a discussion based on Okta system logs in slack, Network zones were considered to be an entity based on the operations performed.
  • Adding a Zone entity to the managed entity type_id enum.
  • Adding a Zone object

Signed-off-by: Martin McCloskey <[email protected]>
Signed-off-by: Martin McCloskey <[email protected]>
Signed-off-by: Martin McCloskey <[email protected]>
@max-power15 max-power15 changed the title Martin.mccloskey/add zone entity Add Zone to managed entity object Sep 25, 2024
Signed-off-by: Martin McCloskey <[email protected]>
@zschmerber
Copy link
Contributor

After looking at a zone.update log in Okta, i seems like the structure of the data my need to be carried by an array, also I see a zone https://schema.ocsf.io/1.3.0/objects/device?extensions= field in the schema so we would have a collision with adding a new object with the same name.

@mikeradka
Copy link
Contributor

Nice draft, @max-power15. I believe adding a Zone entity to the Entity Management class makes a lot of sense. However, there are a few logistics to consider:

  1. OCSF already includes a zone attribute, which is currently of type String. Changing its type to a zone object would be a breaking change.

  2. That said, I do see potential in creating an object which represents a zone (or even an array of zones), as it could include attributes like name and uid, and perhaps additional ones such as a CIDR range. One option could be to introduce a zone_info object or attribute. While the name is flexible, we would need to choose something other than zone to avoid breaking changes.

  3. If we proceed with a new zone_info object (or whatever the name may be), we might consider deprecating the current zone (String) attribute and directing to using the new object.

  4. This could also tie in with the recent discussions around a Network profile. I’d love to hear your thoughts, along with @pagbabian-splunk @floydtree @zschmerber @Aniak5

@max-power15
Copy link
Contributor Author

max-power15 commented Sep 26, 2024

@zschmerber @mikeradka

Great feedback, I wasn't aware of the zone attribute. Happy to adjust the object name and the additional attribute for CIDR

Is there a PR for the network profile I could check out, or is it being discussed in slack?

@mikeradka
Copy link
Contributor

ack, I wasn't aware of the zone attribute. Happy to adjust the object name and the additional attri

Hello @max-power15 , this one slipped through the cracks for me for some reason. This has been more of a discussion topic rather than any sort of PR. Perhaps this is a topic we could discuss in one of the upcoming Network syncs on Wednesdays? It may be worthwhile to send us over a ping about it in the #network slack channel.

@pagbabian-splunk pagbabian-splunk added enhancement New feature or request iam Issues related to Identity & Access Management Category v1.4.0 Changes marked for the upcoming version 1.4.0 labels Jan 21, 2025
"caption": "Zone",
"description": "The Zone object describes characteristics of a network zone, a logical boundary to allow or deny access to devices based on IP addresses or geolocation for example.",
"extends": "_entity",
"name": "zone",
Copy link
Contributor

Choose a reason for hiding this comment

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

The description says that this is specifically a network zone. So I'm thinking that the object should be named network_zone. Otherwise we risk running into a name collision when some other part of the OCSF schema has the concept of a zone but with a very different meaning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto!
If we are planning to get these changes in v1.4.0, the PR will need to updated with what @mikeradka recommended above with a slight change - zone_info can be the object name, and we can define an attribute called network_zone of type zone_info, to keep it clear and facilitate potential object reuse down the road.

@max-power15 Note that, these changes will need to be made by Jan 24th, considering that we are releasing 1.4.0 on Jan 31st. If you are unable to, we can always get the changes in the next minor release.

Let us know if you need help with this one. #schema on slack would be a good forum to discuss it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@floydtree I agree with this approach. It aligns with our discussion yesterday about up-leveling the dictionary attribute. If need be, we could discuss in either of the workgroup calls today. Also happy to help if need be so that we can get this into 1.4. Just let me know!

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. I assume the original zone was in reference to Availability Zone or other zonal boundaries within the CSPs (OCI, GCP, Azure, et al)

I like @floydtree suggestion to adapt

Copy link
Contributor

@mikeradka mikeradka Jan 23, 2025

Choose a reason for hiding this comment

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

@jonrau-at-queryai @jonrau-at-queryai @davemcatcisco @pagbabian-splunk @max-power15 Rajas and I discussed this offline and agreed to target this for 1.5.0. I'm tagging it accordingly, and implementation can begin after the 1.4.0 release, scheduled for Jan 31. This approach allows us to review the PR thoroughly, avoiding rushed changes or potential corrections in 1.5.

@pagbabian-splunk pagbabian-splunk marked this pull request as ready for review January 22, 2025 17:04
@mikeradka mikeradka added v1.5.0 or later and removed v1.4.0 Changes marked for the upcoming version 1.4.0 labels Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request iam Issues related to Identity & Access Management Category v1.5.0 or later
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants