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

feat[list-users]: add excluded users field in list users response #1772

Conversation

lalalalatt
Copy link
Contributor

@lalalalatt lalalalatt commented Jul 16, 2024

Description

As describe in #1692, the excluded user should show explicitly to prevent the confusion on the type-bound public access which doesn't necessarily indicate that all users of that type have access.

This PR required the approval to the openfga/api#179 that modifying protobuf and generated Go file.

Changes

  1. Add an additional field excluded: []string in the response of List User Request, in order to show the users excluded from PublicWildcard.
  2. The listUsersAssertions In the integration test file assets/tests/consolidated_1_1_tests.yaml, can also take the excluded user into expectations setting. With the (excluded) prefix before the user. If there aren't any expected excluded user defined, then the excluded user elements would not be tested.
listUsersAssertions:
  - request:
      filters:
        - user
      object: document:1
      relation: can_view
    expectation:
      - user:*
+     - (excluded)user:bob

References

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

- Add `Excluded` field to `listUsersResponse` struct
- Implement `GetExcluded` method in `listUsersResponse`
- Track the number of excluded users in `ListUsers` function
- Populate `excludedUsers` in `ListUsers` function and include in response
- Return `Excluded` field in `ListUsers` RPC response
Copy link

linux-foundation-easycla bot commented Jul 16, 2024

CLA Signed


The committers listed above are authorized under a signed CLA.

@lalalalatt lalalalatt changed the title feat(list-users): add excluded users field in list users response feat[list-users]: add excluded users field in list users response Jul 16, 2024
- Add user exclusion for Bob in test expectations
- Implement tracking of unique excluded users in ListUsers function
- Adjust excluded users handling in ListUsers function
- Add strings package import in listusers test
- Separate users and excluded users in test expectations and assertions
- Include excluded users in the raw response logs for ListUsers
@lalalalatt lalalalatt marked this pull request as ready for review July 16, 2024 15:52
@lalalalatt lalalalatt requested a review from a team as a code owner July 16, 2024 15:52
@willvedd
Copy link
Contributor

@lalalalatt Thanks for your contribution. I'm going to close this out but will make sure you're aware of any new decisions that are made with respect to excluded users. Please keep an eye out for #1692, that's where I intend the conversation will occur.

@willvedd willvedd closed this Jul 17, 2024
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.

3 participants