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

New session presented event #183

Merged
merged 6 commits into from
Jun 12, 2024
Merged

New session presented event #183

merged 6 commits into from
Jun 12, 2024

Conversation

tulshi
Copy link
Contributor

@tulshi tulshi commented Jun 6, 2024

No description provided.

@tulshi tulshi requested a review from a team as a code owner June 6, 2024 21:04
@tulshi tulshi requested review from iamseanodentity, FragLegs and timcappalli and removed request for a team June 6, 2024 21:04
@FragLegs
Copy link
Contributor

When talking to @iamseanodentity I realized that I think the whole notion of "session presented" is problematic. All other SSF events tell us when something changes. But with session presented, the Receiver would need to compare each new event to stored previous events to determine if things like risk_score, ip, or fp_ua have changed. It would be much more valuable to have RiskScoreChanged, IPChanged, and FingerprintChanged events.

@tulshi
Copy link
Contributor Author

tulshi commented Jun 11, 2024

When talking to @iamseanodentity I realized that I think the whole notion of "session presented" is problematic. All other SSF events tell us when something changes. But with session presented, the Receiver would need to compare each new event to stored previous events to determine if things like risk_score, ip, or fp_ua have changed. It would be much more valuable to have RiskScoreChanged, IPChanged, and FingerprintChanged events.

A few things to consider:

  1. The event is important to build "Identity Threat Detection" capabilities. Such a capability can help receivers understand threats that span various transmitters. Each point-observation may not represent a threat, but together it might constitute one.
  2. The "*changed" events you mention will not convey all possible threats because if an attacker uses the different IP addresses at different applications, but retains the same IP address for each application, then nothing will have changed at an application, and they would not send such "*changed" events.
  3. For receivers who are not interested in such presence events (including those who find them difficult to process, like you describe), they can simply not subscribe to them, so unless you feel this event is not going to help further the CAEP charter, we should keep it.
  4. Finally, the CAEP charter is to improve session security (paraphrasing), and since this event contributes to that charter, I believe it should be added.

@tulshi tulshi requested a review from iamseanodentity June 11, 2024 22:58
@tulshi tulshi requested review from FragLegs and appsdesh June 12, 2024 16:51
@tulshi tulshi requested a review from appsdesh June 12, 2024 17:37
Copy link
Contributor

@FragLegs FragLegs left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Contributor

@appsdesh appsdesh left a comment

Choose a reason for hiding this comment

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

nit: I feel, AMR could be a list as well

Copy link
Member

@timcappalli timcappalli left a comment

Choose a reason for hiding this comment

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

I don't fully understand the use case / need for this event, but it is well formed and I don't think is harmful to include.

@tulshi tulshi merged commit 64229a3 into main Jun 12, 2024
2 checks passed
@tulshi tulshi deleted the session-presence-event branch June 12, 2024 20:11
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.

5 participants