Skip to content

Feat: Super admin - user management #4134

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 95 commits into from
Nov 17, 2022
Merged

Feat: Super admin - user management #4134

merged 95 commits into from
Nov 17, 2022

Conversation

@pranavxc
Copy link
Member

@pranavxc pranavxc commented Oct 19, 2022 •
edited
Loading

Change Summary

  • Root-level user management
    • org-level-creator role - this user can create a new project and access any invited project
    • org-level-viewer role - this user can't create a new project but they can access any invited project
  • Token management
    • Newly created tokens are associated with the created user and it will have all permissions that particular user has
    • Existing token will work as it is and only be visible to the super admin
  • Signup without an invite is disabled by default and can be managed from UI by a super admin.

Change type

  • feat: (new feature for the user, not a new feature for build script)
  • fix: (bug fix for the user, not a fix to a build script)
  • docs: (changes to the documentation)
  • style: (formatting, missing semi colons, etc; no production code change)
  • refactor: (refactoring production code, eg. renaming a variable)
  • test: (adding missing tests, refactoring tests; no production code change)
  • chore: (updating grunt tasks etc; no production code change)

Test/ Verification

Provide summary of changes.

Additional information / screenshots (optional)

Anything for maintainers to be made aware of

@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch 3 times, most recently from 704a9d0 to 7474cd2 Compare October 24, 2022 19:10
@pranavxc pranavxc added the trigger-CI force trigger CI even if PR in draft mode label Oct 25, 2022
@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch 2 times, most recently from 13b8385 to 3dca74d Compare October 25, 2022 05:20
@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch from d9153c8 to bdd07fb Compare October 25, 2022 09:27
@wingkwong wingkwong mentioned this pull request Oct 26, 2022
Closed
1 task
@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch from a5c2a57 to 433d5c8 Compare October 28, 2022 05:59
wingkwong
wingkwong reviewed Oct 28, 2022
View reviewed changes
Copy link
Contributor

@wingkwong wingkwong left a comment •
edited by pranavxc
Loading

Choose a reason for hiding this comment

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

Testing with cache enabled.

    • Will this page be the new page and those in Team & Settings will be removed? or you just test it at this moment?
      image
    • Copy password reset URL
  • the message shows Invite URL copied to clipboard which should be updated
  • the copied value is like http://localhost:3000//auth/password/reset/http://localhost:8080/auth/password/reset/37a51755-aa21-4402-a357-9c0397cebde9
    • The inputted email seems useless? For example, I input w2@nocodb.com, then I use the link to sign in with w3@nocodb.com and it will work too. With such link, I think we don't need to let users to input the email. Just set the password and sign in.
      image
    • Delete user message. remove from from.
      image
    • Cannot re-invite the users that I deleted.
  • invite user w2@nocodb.com
  • delete it
  • invite user w2@nocodb.com
  • failed because user exists while the list doesn't show w2@nocodb.com
    • Invite new user should be rerendered every time it is clicked
  • invited a user. close the window
  • click invite new user again
  • it shows the previous result
    image
    • invite a user. log into nocodb with that user. click the project and shows You don't have enough permission to access the project.
    • super admin is not able to view the new tokens created by users.
      image

@pranavxc
Copy link
Member Author

pranavxc commented Oct 28, 2022 •
edited
Loading

@wingkwong

  1. This new page is for managing users globally(only available for super admin) and differs from project-level user management.
  2. If invite-only signup is enabled only invited users can signup. And if a non-invited user signed up they will have org-level-viewer role by default.
  3. Newly invited users won't have access to any existing project unless the project owner or super admin is invited to the project. If we invite a user as org-level-creator then they can create a new project.

@pranavxc pranavxc requested a review from wingkwong October 29, 2022 09:32
wingkwong
wingkwong reviewed Oct 31, 2022
View reviewed changes
@pranavxc pranavxc requested a review from wingkwong November 1, 2022 06:29
wingkwong
wingkwong reviewed Nov 1, 2022
View reviewed changes
Copy link
Contributor

@wingkwong wingkwong left a comment •
edited by pranavxc
Loading

Choose a reason for hiding this comment

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

  • User Management is not shown by default when Account within a project is clicked

    Select Account from top right -> User Management can be shown (which is expected)
    image

    Go to a project, click the project drop down, select Account
    image

    User Management is not shown by default.
    image

  • User Management tab on menu is not highlighted by default while other tabs do

    On the left hand side menu, User Management tab is not highlighted

    image

    It should be similar to other tabs

    image

  • Users shouldn't be able to access project that they don't belong to

    Steps to reproduce:

    • Create a project Project A by User A
    • User B, who is organization level creator, click the Project A from My Project
    • User B can enter the project but with empty content

    In existing logic, there should be an error message thrown in My Project list when accessing to some unauthorized projects. However, there's some users reporting that the projects that they cannot access shouldn't be shown in My Project.

    image

  • Nice to have a better UI and redirection to Sign In page after resetting password

image

  • After resetting the password, accessing any project would redirect to Sign in page. (related to cache. it is working fine after I clean the cache)

    Steps to reproduce:

    • Reset password
    • Redirect back to sign in page (which is expected)
    • Use the new password to login
    • Choose the project that I could access
    • Redirect back to sign in page (which is not expected)

  • I think it'd look better to use camel case, i.e. Organization Level Viewer and Organization Level Creator

image

@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch 3 times, most recently from f006dd4 to c1c12c4 Compare November 12, 2022 16:27
@pranavxc
Copy link
Member Author

pranavxc commented Nov 14, 2022

Nice to have a better UI and redirection to Sign In page after resetting password

@wingkwong: will do it in a separate PR

@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch from 3eba2b2 to 145e5ac Compare November 14, 2022 10:12
@pranavxc pranavxc requested a review from wingkwong November 14, 2022 10:12
@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch from 145e5ac to 7cb92d6 Compare November 14, 2022 19:29
wingkwong
wingkwong reviewed Nov 15, 2022
View reviewed changes
@pranavxc pranavxc requested a review from bcakmakoglu November 15, 2022 13:34
@pranavxc
fix(gui): correction in if condition
99ee8a0 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
refactor(playwright): remove only from test
22a9c0e 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
refactor(gui): update label
1704cfd 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
fix(gui): show error message if user tried to access unauthorized pro…
b110b94 
…ject

Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
fix(gui): clear the email based user cache in update to avoid unexpec…
28aae45 
…ted behaviour

Scenarios where it could fail

- If user update is missing email id
- If user update includes a different email id than existing

Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
test(playwright): improve invite only signup settings test
389f576 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
test(playwright): signup and verify create project button exist or no…
ede2ca2 
…t based on role

Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
token toggling
90de1e6 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
refactor: corrections and requested changes
5785f05 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
test(playwright): signout before going to signup page
aa3a0fe 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc
refactor: linting and requested changes
b1b28e1 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch 4 times, most recently from 38d5fee to 81a14b6 Compare November 16, 2022 18:27
@pranavxc
test(playwright): update apps page link
4f7d8a4 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch from 81a14b6 to 4f7d8a4 Compare November 16, 2022 18:47
@pranavxc
test(playwright): reload page to handle route navigation error
1ee30f4 
- TypeError: Cannot read properties of null (reading 'parentNode')

Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc pranavxc force-pushed the feat/super-admin-user-management branch from 980e845 to 1ee30f4 Compare November 17, 2022 05:43
@pranavxc
test(playwright): wait until meta api call completes
9f249d0 
Signed-off-by: Pranav C <pranavxc@gmail.com>
@pranavxc pranavxc marked this pull request as ready for review November 17, 2022 07:14
wingkwong
wingkwong reviewed Nov 17, 2022
View reviewed changes
Copy link
Contributor

@wingkwong wingkwong left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The below testings are conducted without enabling cache.

  1. Text size not aligned
    image

  2. Initial load shows empty content for all tabs (works fine after refresh)
    image

  3. Project List is gone after accessing a project that is not accessible. (IMO: inaccessible projects shouldn't be shown)

image

Sometimes it shows like

image

  1. Outdated permissions still retain as long as they don't log out.

steps to reproduce:

  • invite a user w1@nocodb.com with role Organization Level Viewer using super admin
  • log into w1@nocodb.com in different browser
  • change w1@nocodb.com to Organization Level Creator using super admin
  • go back to w1@nocodb.com browswer, refresh the page, it's still using Viewer permission.

This applies to Organization Level Creator <==> Organization Level Viewer. Suggest once the role is changed, invalidate the session so that users need to force login again. (Just like after changing password).

  1. Empty Project shown after renaming project

image

image

steps to reproduce: (using Creator role)

  • create a project xcdb2
  • create a table in xcdb2
  • go back to My Projects and rename xcdb2 to xcdb3
  • it shows an empty project
  • refresh it. all data is back.
  1. w@nocodb.com is the super admin account. Should it be hidden in Users Management for Creator roles? or Under Role, we may show it is a super admin.

image

  1. The view is messed up. Not sure how to reproduce this but I had encountered several times when testing. Maybe try accessing Account settings from the project.
    image

@o1lab o1lab merged commit 4943c1d into develop Nov 17, 2022
@o1lab o1lab deleted the feat/super-admin-user-management branch November 17, 2022 12:54
@pranavxc pranavxc mentioned this pull request Nov 18, 2022
Merged
7 tasks
@wingkwong wingkwong mentioned this pull request Nov 19, 2022
Closed
1 task
wingkwong added a commit that referenced this pull request Nov 22, 2022
@wingkwong
docs: account settings
dc9f7cc 
ref: #4134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@wingkwong wingkwong wingkwong left review comments

@bcakmakoglu bcakmakoglu bcakmakoglu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@wingkwong wingkwong

@bcakmakoglu bcakmakoglu

Labels

trigger-CI force trigger CI even if PR in draft mode

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pranavxc @wingkwong @bcakmakoglu @o1lab