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

Empty ref node calculation does not work for canonical 'User.groups[].type' value #176

Closed
hamiltont opened this issue Jun 17, 2021 · 5 comments

Comments

@hamiltont
Copy link
Contributor

Running version 1.10.0.

Issue

The feature of adding a $ref value is super handy. Unfortunately it does not seem to work with the canonical values for listing groups a user belongs to, because it uses the RFC7643.TYPE field to try and lookup a resource type. This fails if you use the RFC7643 recommended values of "direct" or "indirect". Seems like a simple fix is possible - add a special case to check if the TYPE field is one of the canonical values of "direct" or "indirect", and map that to "Group".

For example:

    "groups": [
        {
            "value": "9d66400a-149e-40da-b55a-96d9bb6100aa",
            "display": "test group",
            "type": "indirect"
        }
    ],

Workaround

Use non-canonical resource name e.g. GroupNode.builder().type("Group") instead of GroupNode.builder().type("direct"). This works as expected:

    "groups": [
        {
            "value": "9d66400a-149e-40da-b55a-96d9bb6100aa",
            "$ref": "http://localhost:8089/scim/v2/Groups/9d66400a-149e-40da-b55a-96d9bb6100aa",
            "display": "test group",
            "type": "Group"
        }
    ],

RFC7643 section 4.1.2

groups
A list of groups to which the user belongs, either through direct
membership, through nested groups, or dynamically calculated. The
values are meant to enable expression of common group-based or
role-based access control models, although no explicit
authorization model is defined. It is intended that the semantics
of group membership and any behavior or authorization granted as a
result of membership are defined by the service provider. The
canonical types "direct" and "indirect" are defined to describe
how the group membership was derived. Direct group membership
indicates that the user is directly associated with the group and
SHOULD indicate that clients may modify membership through the
"Group" resource. Indirect membership indicates that user
membership is transitive or dynamic and implies that clients
cannot modify indirect group membership through the "Group"
resource but MAY modify direct group membership through the
"Group" resource, which may influence indirect memberships. If
the SCIM service provider exposes a "Group" resource, the "value"
sub-attribute MUST be the "id", and the "$ref" sub-attribute must
be the URI of the corresponding "Group" resources to which the
user belongs. Since this attribute has a mutability of
"readOnly", group membership changes MUST be applied via the
"Group" Resource (Section 4.2). This attribute has a mutability
of "readOnly".

Relevant code

See ResponseAttributeValidator#overrideEmptyReferenceNodeInComplex:

@Captain-P-Goldfish
Copy link
Owner

I see your point but I would need to think of a reliable solution. The custom case you mentioned is not an option for this implementation. The type could be used with values of "direct" and "indirect" in other custom resources too this would create a conflict causing reference urls of Group-resources to be entered in other resources.
Would it be possible for you to define a custom attribute in the MEMBER-attribute that you use do place the "direct" and "indirect" values?

Alternatively I might be able to add a Function in the ResourceHandler supplier that you might use to add the url reference manually:

getResourceReferenceUrl.apply(resourceId)

what do you think? would this be an appropriate solution for you?

@Captain-P-Goldfish
Copy link
Owner

I just noticed that the previous noted solution with the Function is not so easily achieved. This would require an API change on the method structure of the ResourceHandler.

@Captain-P-Goldfish
Copy link
Owner

Please take a short look at the changes. I changed the method signature on the ResourceHandler methods. They will receive a Context object now that provides the Authorization object that was previously in the method signature itself and also 2 methods that should provide you with convenience functionality that should meet your requirements.

The references are not added automatically as you would have liked but I fear I have no better solution here.

@hamiltont
Copy link
Contributor Author

Oh, I much prefer the Context based approach. In the future it would be much simpler to expand effectiveContext with new variables/objects if other feature requests come in.

It looks like it would force a code update on all users of the current API, (which I am fine with!) - that could be mitigated with some default facade methods that accept the new Context and by default call the corresponding 'auth-only' legacy methods. I personally do not need this, just mentioning it as a possibility

Thanks for the quick response!

@Captain-P-Goldfish
Copy link
Owner

I actually wanted to this for quite some time now since I was very unhappy with the Authorization-interface that was used as context so far.
but great I consider this fixed then.

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

No branches or pull requests

2 participants