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

Provide a definition of wlcg.groups that is independent of VOMS. #15

Closed

Conversation

bbockelm
Copy link
Contributor

This is a replacement for #2; it removes the text about implicit authorizations and makes a strong requirement for explicit-only group authorization.

@hannahshort
Copy link
Contributor

Thanks, do you know whether it's possible to fix the link?

@bbockelm
Copy link
Contributor Author

I think I fixed it?

@maarten-litmaath
Copy link
Collaborator

Hi all,
as the discussion in #2 had not converged yet, we may want to discuss this PR in an AuthZ WG call (e.g. on Dec 16) before merging it.

@norealroots
Copy link
Collaborator

Hi all, as the discussion in #2 had not converged yet, we may want to discuss this PR in an AuthZ WG call (e.g. on Dec 16) before merging it.

I can drop this onto the agenda for the 16th now


Relying parties MUST only consider the group membership asserted in the token for authorization decisions. Particularly, membership in a child group (`/cms/uscms`) does not implicitly imply membership in the parent group (`/cms`) unless the parent group is explicitly listed in the token.

The token does not need to list all groups the user has access to; relying parties MUST utilize only the asserted groups in the implied order. The `wlcg.groups` claim is optional and may either be empty or not provided at all. The user may provide input on the contents and ordering of this claim; this is covered [below](#scope-based-attribute-selection).

Choose a reason for hiding this comment

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

The token does not need to list all groups the user has access to

I may have missed past discussions, but what is the rationale? What problem are you trying to solve wrt to the status quo?

In any case, the section "Scope-based Group Selection" would also need to be rewritten.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The rationale would be that tokens can/should be made as lean as possible to:

  1. mitigate potential concerns about their sizes (there could be a large number of groups);
  2. do not bring contents that are totally irrelevant to the authZ decision (but have to be parsed, forwarded and logged nonetheless).

Choose a reason for hiding this comment

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

It sounds as premature optimization to me, but at least it's clear now. This requires changes in the token issuer, that's why I asked to rewrite also the following section.


Relying parties MUST only consider the group membership asserted in the token for authorization decisions. Particularly, membership in a child group (`/cms/uscms`) does not implicitly imply membership in the parent group (`/cms`) unless the parent group is explicitly listed in the token.
Copy link
Contributor

Choose a reason for hiding this comment

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

Relying parties MUST only consider the group membership asserted in the token for authorization decisions

This sentence seems to rule out AuthZ decisions based on "scope"-claim values ("MUST only consider ... for authz decisions")

I think what you meant to say is something like:

Relying parties MAY consider the subject as being a member of the various groups asserted by the
token; however, such relying parties MUST NOT infer membership of any other groups that are not
asserted by the OP.

Like this, I don't think you need the specific example.

Copy link

@msalle msalle Feb 1, 2023

Choose a reason for hiding this comment

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

Reformulating that a bit you can keep the original meaning (i.e. keeping the authZ decisions), but using your negative (MUST NOT) which is indeed better:

For authorization decisions relying parties MUST NOT consider any group memberships that are not
asserted by the OP in the token.

maarten-litmaath added a commit that referenced this pull request Jan 2, 2024
Consolidation of several changes based on: 
#15, 
comments therein plus minor further adjustments.
@maarten-litmaath
Copy link
Collaborator

Hi all,
I propose to have this PR superseded by #46 instead.

@maarten-litmaath
Copy link
Collaborator

Superseded by #46 .

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

7 participants