Skip to content

Updating the context response object + pagination: adding support for… #341

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

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

Conversation

baboulebou
Copy link
Collaborator

Modifying and cleaning up context response object… offset pagination.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @baboulebou ! Ideally this would be split across two PRs since there are two distinct issues addressed here, that each have different considerations.

I will review in more detail, but my initial impressions are that the context clarification could perhaps better fit in a profile? Certainly things like “obligations” are fairly PDP-specific, and I could see that in an ABAC profile.

I think that at the syntax level (which is what AuthZEN 1.0 focuses on) we should have an extensible response context mechanism that can be used for many things (reason codes, obligations, step up, etc) without having to specify all of them in the core spec.

As far as pagination, would having a page_token and an offset be enough to satisfy both use-cases (with no need to specify the cursor type?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, I'm afraid you'll have to live with 1 PR for both for @ogazitt , I'm already stretched for time, but hopefully we can still gather comments for the pagination part. About that, Cursor and Token pagination are the same thing, are they not? I could rename "token" for "cursor", but then we've already published drafts with "token".

For obligations, figured we could add it here, we (Indykite) actually use a similar construct already to signal Step-up...an obligation. We could adopt a profile strategy but really the proposal here covers all the comments/issues I've seen raised here. Having a bit more structure in context should help interoperability...

Anyway, hope you guys can discuss this this week, being in Europe I will miss the weekly this week.

where:

`reason-code`:
: REQUIRED. A String value representing a code that uniquely identifies the error. Implementations that do not use any error-code mechanism SHOULD default to valid HTTP Error codes (see {{RFC7231}} ).
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does the reason-code relate to the id in the reason object? It seems like the id is some arbitrary number, whereas the reason-code is meant to be an HTTP error code. Is there a reason why we need both?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually mentioned this in the description of the id field:

"In case the PDP returns several reasons, the id MUST uniquely identify each one of them."

The "reason-code" specifies a code for a given reason. Since we're specifying a reason-admin and a reason-user, those may even have different codes, that could be implementation specific. For example, reason-admin could be a 403, but reason-user could be "something else. Just thought it could provide more flexibility.

An alternative would be to get rid of id, move reason-code to the top - the same code applies to all messages- and just have 2 simple (non-complex) properties: reason-admin and reason-code with simple string values. Would that be better/sufficient?


### Sample Response with additional context (non-normative)
#### Environment
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, we defined "context" in the request to provide (among other things) environmental attributes. Why are we calling these out specifically?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, if indeed "context" meant environmental considerations (you know, the "context" :) ), it seems that people have been willing to cram all sort of things in "context", including information that one wouldn't necessarily consider "context". So calling out an "environment" field here would make things more explicit and maybe easier for interoperability ?

"reason": {
"id": "0",
"reason_admin": {
"insufficient_user_authentication": "Request failed policy C076E82F"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought these were supposed to be HTTP codes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, I stated in the "reason" section higher up, that the reason-code is:

": REQUIRED. A String value representing a code that uniquely identifies the error. Implementations that do not use any error-code mechanism SHOULD default to valid HTTP Error codes (see {{RFC7231}} )."

Rewording to (adding "custom"):

": REQUIRED. A String value representing a code that uniquely identifies the error. Implementations that do not use any custom error-code mechanism SHOULD default to valid HTTP Error codes (see {{RFC7231}} )."

Would that clarify things? I.e., suggesting that we use HTTP codes by default, but could also use custom ones. Is that too permissive?

}
}
],
"metadata": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these should just be part of "properties"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes maybe. But again here separating it may make things a bit more interoperable?

Offset:
: Pagination is performed by passing two parameters along with each Search request: `offset`, which points to the beginning record for the current page, and `limit`, which determines the number of records to return starting from the `offset` record (i.e., the page size). In this case, the PEP requests a specific set of records.

## Pagination semantics
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pagination is PEP/storage implementation-specific; therefore, this will not help the clarity or simplicity of the API.

Why should the consumer of the AuthZEN API need to understand or specify a pagination semantic?

Doing so goes against the objective of the API specification to provide an authorization API whicPagination is specific to the implementation of PEP/storage; therefore, it does not enhance the clarity or simplicity of the API.

Why should users of the AuthZEN API need to understand or define pagination semantics?

Requiring this knowledge contradicts the goal of the API specification, which is to provide an authorization API that is independent and agnostic of any specific implementation.

We use a page size that can only be requested, as you do not want to make this change while fetching. We use a base64-encoded token to handle special characters, case-sensitivity, and additional complex information, allowing for continuation while remaining agnostic to the storage system used underneath.

If PEP / storage-specific aspects are leaking into the API, we are missing the objective of the API.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Authzen defines 3 search apis, each of which may return a numerous results. The Resource Search API for example, should return all the resources of a certain type that the user has access to. There may be thousands of them, therefore the Search API must support some form of pagination, and the PEP should paginate results, no way around it. The spec defines the search APIs, I don't see how leaving the pagination out of it would help here? I can't even imagine how a PEP could paginate if the API itself doesn't support it... the PEP would have to use another search API that supports pagination...

This PR adds an offset-limit semantic to the originally proposed token pagination (which is still there, and which seems in-line with what you describe @gertd ). The offset/limit pagination method is popular for database systems too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The point remains that the spec does not call out that the next token:

  • Regardless of its type, it is only valid for the continuation of the request response that returned the next token
  • As such, the token shape and content are NOT part of the API specification and therefore cross-compatibility between implementations, as it is a runtime implementation detail.
  • The only thing that is part of the API specification is that a next token is a non-null string.

The Reason object being a JSON object, implementations MUST follow the following structure; the `reason` context contains the following elements:

`id`:
: REQUIRED. A string value that specifies the reason within the scope of a particular response. In case the PDP returns several reasons, the `id` MUST uniquely identify each one of them. The `id` MAY also match specific reasons held by the PDP, part of a reasons list or dictionnary for example, and used for audit correlations.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The way I read this, the "id" returned is a PEP-specific "id"; how is this in line with the objective of the specification of being implementation agnostic? It would be better if the specification defines the IDs similar to gRPC status codes and HTTP status codes.

Copy link
Collaborator Author

@baboulebou baboulebou Jul 7, 2025

Choose a reason for hiding this comment

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

This comment seems to be a duplicate of @ogazitt 's comment (see further up). Sounds like we need to discuss the Reasons some more.

I'm proposing this simplification:

  • no id, just a reason-code.
  • the reason-code could be an HTTP error code. Now, because I assume that only 401, 403 or 404 would make sense here, these may not be sufficient to express implementation-specific error conditions (e.g., "insufficient funds in bank account"), therefore:
  • the reason-code could also be an implementation-specific custom code (e.g., " insufficient_funds ").

The reason object could thus look like this:

[
  {
    "reason_admin": {
      "403": "Request failed policy C076E82F"
    },
    "reason_user": {
      "insufficient_funds": "Insufficient funds in bank account."
    }
  }
]

Is this acceptable to everyone?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am in favor of a reason-code instead of an id, as this could also remove any localization aspects of the string representations of reasons and make it easier inside a code base to reason about code programmatically, rather than interpreting a string representation.

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.

4 participants