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

First iteration of Event Def Spec #209

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

Conversation

jischr
Copy link
Contributor

@jischr jischr commented Sep 24, 2024

This PR addresses #158

@jischr jischr requested a review from a team as a code owner September 24, 2024 16:52
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved

1. Author(s) of the pull request MUST be at least a contributing member of the OpenID Foundation.
1. The Pull Request MUST contain a human readable description of the new SSF event type.
1. The $id of the schema MUST be publicly accesbile on the internet and resolve to the schema document.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to worry about broken or stale URLs? Should there be a process for handling those?

openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
1. The $id of the schema MUST be publicly accesbile on the internet and resolve to the schema document.
1. The "title" and "description" of the schema must be meaningful and indicative of its function.
1. The naming of all schema properties MUST be indicative of its function.
1. Schemas may not be removed, only deprecated. Any changes to schemas must follow semantic versioning.
Copy link
Contributor

Choose a reason for hiding this comment

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

How will we mark schemas as deprecated?

Copy link
Contributor Author

@jischr jischr Oct 8, 2024

Choose a reason for hiding this comment

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

schemas cannot be deprecated explicitly but superseded by newer versions
[edit] : im actually not sure about this... lets discuss today

openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
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.

Initial pass before today's call.

openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved

This section serves as a registry for the schemas of all registered SSF Event Types.

| Event Type | Schema URI | Description
Copy link
Member

Choose a reason for hiding this comment

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

My goal with this effort was to eliminate the need for this classification. An event should be descriptive enough to not need a bucket.

~~~ json
{
"$schema":"https://json-schema.org/draft/2020-12/schema",
"$id":"https://schemas.openid.net/secevent/caep/event-type/session-revoked/v1.schema.json",
Copy link
Member

Choose a reason for hiding this comment

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

I think we might need to define a mapping of the event identifier in the SET to the JSON schema identifier.

E.g. the event identifier is $id with .schema.json stripped?

And then we can say the inverse: an event's schema can be found by appending .schema.json to the event identifier and then dereferencing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think thats a good idea.

openid-sharedsignals-eventdef-specification-1_0.md Outdated Show resolved Hide resolved
Comment on lines 260 to 262
1. Author(s) of the pull request MUST be at least a contributing member of the OpenID Foundation.
1. The Pull Request MUST contain a human readable description of the new SSF event type.
1. The $id of the schema MUST be publicly accesbile on the internet and resolve to the schema document.
Copy link
Member

Choose a reason for hiding this comment

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

An alternative is that we use issues with issue templates, so we can require certain information be provided.

jischr and others added 2 commits October 8, 2024 08:04
Co-authored-by: Tim Cappalli <[email protected]>
Co-authored-by: Shayne Miel (he/him) <[email protected]>
Co-authored-by: Tim Cappalli <[email protected]>
Co-authored-by: Shayne Miel (he/him) <[email protected]>
@tulshi
Copy link
Contributor

tulshi commented Oct 10, 2024

This is a great start, @jischr ! I would like to understand how a collection of events can be defined and identified as such here. For example, if someone wanted to implement "session security", then there may be many events that come under that collection (today named CAEP). Would there be a way to identify that in this scheme?

@jischr
Copy link
Contributor Author

jischr commented Oct 10, 2024

This is a great start, @jischr ! I would like to understand how a collection of events can be defined and identified as such here. For example, if someone wanted to implement "session security", then there may be many events that come under that collection (today named CAEP). Would there be a way to identify that in this scheme?

@tulshi We could refer to this "Collection" in a new schema that described the whole events array.
It could then require two items of specified schemas - lets say 2 events total one of event type A and one of event type B.
It could also require only one event : either event type A or event type B.
Or any combination of schemas.

I think the big takeaway from your comment is the distinction between event definitions and collections of event definitions.

Let me rough draft how that could look!

@timcappalli
Copy link
Member

timcappalli commented Oct 10, 2024

For example, if someone wanted to implement "session security", then there may be many events that come under that collection (today named CAEP). Would there be a way to identify that in this scheme?

Wouldn't this just be addressed via an interop profile? Each event should be able to stand on its own in terms of understanding its usage. I don't think we should formally group events in this new model.

We already have risc or caep in the event name / path, which will ultimately need to be folders in the repo. Do we need more than that?

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.

6 participants