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

fix(auth): remove add to ws in user creation #5686

Merged
merged 1 commit into from
Nov 15, 2021
Merged

fix(auth): remove add to ws in user creation #5686

merged 1 commit into from
Nov 15, 2021

Conversation

EFF
Copy link
Member

@EFF EFF commented Nov 11, 2021

Description

Users were "created" while being added to allowed workspaces but users were created first, meaning we created users twice. I simply moved the code to pro auth-strategies so we get the expected behaviour

Created a docker image, deployed on an instance, configured auth strategies, created users and single user was created as of before it was doubled. (doubly checked by @sebburon )
If you want to try it yourself, pull effit/bp:signon docker image on docker hub

Fixes botpress/v12#1550
Closes DEV-2007

  • Bug fix (non-breaking change which fixes an issue)

@linear
Copy link

linear bot commented Nov 11, 2021

DEV-2007 [BUG] New admins are added as developers (botpress/borpress botpress/v12#1550)

Describe the bug
When adding a new admin collaborator to a workspace, that user is also added as a developer and cannot see

To Reproduce

  1. Deploy an instance of Botpress with Docker
  2. Go to "Collaborators" and add a new collaborator with the "Administrator" role
  3. The new user is added as bot an admin and a developer

Expected behavior
The added user should be an admin only.
Screenshots

Environment (please complete the following information):

  • OS: Docker
  • Browser Chrome
  • Botpress Version 12.26.6 and 12.26.7

botpress/borpress botpress/v12#1550 by @ Gordon-BP

@EFF EFF requested a review from allardy November 11, 2021 16:51
@sebburon
Copy link
Contributor

I tested this, and it works.

@allardy
Copy link
Member

allardy commented Nov 12, 2021

Actually the user was not created twice, but he was added twice to the workspaces (when you create the user manually, he's created with the default role & the one you selected). I see two scenarios possible:

  1. Adding a user via the collaborator page on the admin panel. This email/strategy will be added to the specified workspace only, with the selected role. If a user is added manually once, he will not be added to the other workspaces when he logs on the first time
  2. A user logs on with a configured IDP. We add that user to each workspaces configured with that strategy with the default role of that workspace

I'm not sure if there's other scenarios, but overall LGTM

Copy link
Member

@allardy allardy left a comment

Choose a reason for hiding this comment

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

LGTM

@EFF EFF merged commit 6ed8f6d into master Nov 15, 2021
This was referenced Dec 2, 2021
@EFF EFF deleted the f_fix-double-user branch January 18, 2022 17:24
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.

[BUG] New admins are added as developers
4 participants