Closed AbegaM closed 6 months ago
There is a general issue in this PR where quotes are being corrected. IMHO this is a legacy issue in soul
. We could do a separate PR to fix all of the quotes in the app. Then when we update our PR(s) they will inherit the fixed quotes. But, I suspect that will cause lots of painful conflicts. So, I think we'll have to live with this issue until we can merge this stuff back into develop
. Then we can make the quotes consistent.
There is a general issue in this PR where quotes are being corrected. IMHO this is a legacy issue in
soul
. We could do a separate PR to fix all of the quotes in the app. Then when we update our PR(s) they will inherit the fixed quotes. But, I suspect that will cause lots of painful conflicts. So, I think we'll have to live with this issue until we can merge this stuff back intodevelop
. Then we can make the quotes consistent.
Yes, this issue needs to be fixed, we can work on it after finishing the authentication feature
Hi There is an error while creating a super user. Because once we create the super user, make some changes to the code, and save it, By saving the changes, it tries to create a supper user again, which causes the app to crash. So we should need to validate that once the super user is already created, do not try to create the super user again.
Hi There is an error while creating a super user. Because once we create the super user, make some changes to the code, and save it, By saving the changes, it tries to create a supper user again, which causes the app to crash. So we should need to validate that once the super user is already created, do not try to create the super user again.
Hi @TahaKhanAbdalli , Could you please clarify the issue? because we are not creating a super user from the CLI
, what you do is you first create a user via the /api/tables/_users/rows
API, and then you update the user you created to a super user from the CLI
Basically I used this command to run the server. When I do yarn cli
or yarn start --database chinook.db --port 8000 createsuperuser --username john --password 123456789
by passing in flags to create user it creates the db
and creates the super user with given credentials. Whenever I restart the server it tries to re-run to create the user again which already exists and it causes the error something like user already exists
and server crashes. I think instead of crashing the server it should check if user already exists then it shouldn't trigger the query to create user.
Hello @Muhammad Taha Khan , Could you please check this Readme file, i see you are passing a flag createsuperuser
in the cli
script but we dont have that script in the new PR, Could you please let me know which branch you are using for the test?
I was using 138_authentication
branch for testing.
I was using
138_authentication
branch for testing.
Hello @TahaKhanAbdalli - we're sequencing the PRs by each feature being in it's own branch, in it's own PR. So, you'll see this PR has its own branch.
@IanMayo Do you think it is better if we can change the updateuser
CLI command to updatesuperuser
? because we have said that we are using this command only to update the password
and the is_superuser
status of superusers but not normal users
Yes - that would prevent soul users thinking the command is for updating normal users.
Note: the tidying up in https://github.com/thevahidal/soul/tree/authentication_and_authorization
has introduced a conflict that we need to fix in this branch. But, the conflict is quite understandable, and easy for us to fix. Note, once we update it in this branch, we may need to resolve it in downstream branches, but we may not. Nevertheless, it is worth promptly resolving the conflict in this PR, the conflicts only get more difficult to fix, if we leave them.
@thevahidal and @IanMayo The code is fixed based on the provided comments, @thevahidal Let us know if you have any other comments on this PR
Hello @thevahidal All of your new three comments in this PR are fixed, since the checkPasswordStrength
function is in the next PR, i have called it in the controller function and we will be good when the next branch is merged into this one
Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Our GitHub checks need improvements? Share your feedbacks!
Modifications
cli.js
file and added a new CLI commandupdateUser