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

Detect organization membership by roles instead of scanning the member list #946

Closed
wants to merge 1 commit into from

Conversation

kherock
Copy link

@kherock kherock commented Apr 10, 2024

Fixes #925. The deprecation and removal of the roles field for version 1.0.0 came with a bug that prevented organization members from being added to organizations with 50 or more members.

The current code only finds members if they appear in the first 50 members of an organization. This corresponds to the default page size of 50 in the Auth0 Go SDK.

🔧 Changes

Rather than scanning through many pages of members, this PR restores some of the old behavior of checking whether the user has any roles associated with their organization membership. If Auth0 returns a 404 on this endpoint, it means that the user is not a member.

📚 References

🔬 Testing

I've tested these changes in my own fork and they work as expected - currently I do not have much time to dedicate on adding tests. I hope that due to the nature of this bug, this fix should be acceptable as long as it doesn't cause any regressions 🤞😬

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@developerkunal
Copy link
Contributor

Hey @kherock,

I hope you're doing well and having a nice day!

I apologize for the delayed response.

I just wanted to provide you with an update regarding your PR (#946). While we initially considered your approach, upon further review, we realized that it might be a bit of a workaround. Your idea of checking for specific roles to determine organization membership is a bit indirect.

Instead, we've decided to pursue a different approach, as outlined in PR (#961). This new approach involves using checkpoint pagination through the members endpoint, which we believe is a more direct and consistent solution.

Given this direction, since we've already merged a similar solution in PR (#961), we won't be merging your PR. However, I want to sincerely thank you for your contribution and effort.

If you have any questions or concerns, feel free to reach out. Thanks again for your contribution!

Best Regards,

@kherock
Copy link
Author

kherock commented May 28, 2024

Thanks for looking at this -
I'm not sure if I agree with this approach being indirect, roles are an intrinsic property of every member. It also performs significantly better for projects that only declare one or two members in a large organization.

The approach in the other PR does not perform well with organizations containing many members - I would only use that for a Terraform resource maintaining an exhaustive list of all members.

E.g. of an organization has 10 pages of members before any Terraform-managed members are added, those 10 pages will have to be fetched for every single member added!

@developerkunal
Copy link
Contributor

Hi @kherock,

I hope you're having a good day!

I wanted to thank you for your input; your contribution means a lot to us. I understand your point, but we want to make sure we're using the proper members endpoint instead of just depending on member roles to determine if a user is part of an organization. If you have any questions or concerns, please don't hesitate to ask.

Thanks again!

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.

Can't import or create auth0_organization_member on Organizations with more than 50 members
2 participants