-
Notifications
You must be signed in to change notification settings - Fork 11
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
[Bug]: Ensure user is not created as admin first before the update #12
Comments
I like the approach following least privilege first :) we check whether we will release it as an update or take care of it in our already started 6.5 refactoring process. |
I have not checked the code but is this still an issue with 6.0 or has this actually been fixed? |
@JoshuaBehrens Any news when this might be fixed? As currently one could use password forget if the update fails and the user has full admin permissions. it should also be a simple fix, just add 'admin' => false to https://github.com/HEPTACOM/HeptacomShopwarePlatformAdminOpenAuth/blob/main/src/Service/UserResolver.php#L66 |
@AndreasA as @wimwenigerkind already mentioned: in the version 7 of the plugin this is fixed by having a different approach on managing roles at all. So this could be the way for you to work with it. |
Hi @JoshuaBehrens the issue still exists in theory:
As mentioned it is a bit of an edge case but the fix is quite simple, just call $this->userProvisioner->provision($user->primaryEmail, $password, ['admin' => false, 'email' => $user->primaryEmail]); instead of $this->userProvisioner->provision($user->primaryEmail, $password, ['email' => $user->primaryEmail]); I can create a PR for this. Would also like to see that change in 6.x for extended support Shopware versions. Maybe even |
Plugin Version
4.2.1
PHP Version
8.1
Shopware Version
6.4.18.1
Installation method
Composer
Identity provider
Google Cloud
What happened?
The shopware user provisioner creates a user as admin by default. Therefore, the user is first created as admin - as nothing else is specified.
It would probably be better to initially create the user as non admin there and only use updateUser to set admin to true, if specified accordingly.
Relevant log output
No response
The text was updated successfully, but these errors were encountered: