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

feature: Implement Admin API User functions and Refactor Test Suite #195

Merged
merged 11 commits into from
Dec 20, 2021

Conversation

dthyresson
Copy link
Contributor

@dthyresson dthyresson commented Dec 15, 2021

What kind of change does this PR introduce?

The PR is twofold:

Fixes #190
Fixes #191
Fixes #187

1 - Implements Issue #190 to allow admin api to

  • get the user by an id
  • update a user by id (with both user and app metadata)
  • delete a user (but now using the service not api client, not the auth client when a service role JWT)

What is the current behavior?

2 - The test suite
a) lacked coverage
b) was confusing to see coverage because was scenario driven (sign up enabled/disabled) and mixed auth and service api methods in the tests)
c) did not work well with test tools like [Wallaby[(https://wallabyjs.com) as the test had to be run sequentially due to the way emails and password were generated to create test users

What is the new behavior?

  1. Implements the needed admin functions
  2. Centralizes client and client config to promote reuse
  3. Utility functions mock credentials (email, password, and phone) to reuse
  4. Refactors suite to tell more "stories" and work with Wallaby testing

GoTrueApi

Screen Shot 2021-12-14 at 6 57 39 PM

GoTrueClient

Screen Shot 2021-12-14 at 6 54 57 PM

Test Results

Screen Shot 2021-12-14 at 6 55 40 PM

Additional context

Note: May still need some more enabled/disabled scenario tests and also have one disabled verifyOTP() test

@dthyresson dthyresson self-assigned this Dec 15, 2021
@dthyresson
Copy link
Contributor Author

dthyresson commented Dec 15, 2021

I got a message that a secret was exposed here in my clients as GOTRUE_JWT_SECRET

image

But this same value is in the docker compose needed for infra local deployment, so I am not that concerned.

@dthyresson dthyresson changed the title feature: Implement Admin API User functions and refactorsTest Suite feature: Implement Admin API User functions and Refactor Test Suite Dec 15, 2021
@dthyresson
Copy link
Contributor Author

Once the PR is approved will need to update the admin docs for:

  • getUserById
  • updateUserById
  • deleteUser

@dthyresson
Copy link
Contributor Author

Note: the listUsers command perhaps should be renamed findUsers since I just learned that the GoTrue implementatiion supports a find filter using FindUsersInAudience that finds users with the matching audience with a filter option and a sort:

	if filter != "" {
		lf := "%" + filter + "%"
		// we must specify the collation in order to get case insensitive search for the JSON column
		q = q.Where("(email LIKE ? OR raw_user_meta_data->>'full_name' ILIKE ?)", lf, lf)
	}

	if sortParams != nil && len(sortParams.Fields) > 0 {
		for _, field := range sortParams.Fields {
			q = q.Order(field.Name + " " + string(field.Dir))
		}
	}

But, the only filter currently is email.

I'll make a separate issue for adding filter support to listUsers or findUsers.

Copy link
Contributor

@thorwebdev thorwebdev left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Regarding #195 (comment) I think it's fine to keep it named listUsers and add the filters in there e.g. listUsers({ email, limit }) etc.

@dthyresson
Copy link
Contributor Author

@kangmingtay Made some mods and tried to answer your questions and hope resolved so can be merged. Thanks!

@dthyresson dthyresson merged commit d7b334a into master Dec 20, 2021
@github-actions
Copy link
Contributor

🎉 This PR is included in version 1.22.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@thorwebdev thorwebdev deleted the dt-user-admin-190 branch May 9, 2022 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants