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

Remove supportOldDangerousPassword support #867

Conversation

omerimzali
Copy link
Contributor

@omerimzali omerimzali commented Sep 30, 2023

Fixes #771

Ref: ea9688d

Description
I removed supportOldDangerousPassword. And also there is no usage of verifyDanger() anymore I also deleted it. I didn't update the docs because docs already have the information about deprecation for version 1.0.0 release.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis changed the title supportOldDangerousPassword support removed Fixes #771 Remove supportOldDangerousPassword support Sep 30, 2023
@kenjis kenjis added the GPG-Signing needed Pull requests that need GPG-Signing label Sep 30, 2023
@datamweb datamweb added GPG-Signing needed Pull requests that need GPG-Signing and removed GPG-Signing needed Pull requests that need GPG-Signing labels Sep 30, 2023
@datamweb
Copy link
Collaborator

datamweb commented Sep 30, 2023

@omerimzali,
Please fix phpStan by changing below to 8:

'count' => 9,

Also, run the following command to fix the code style:

composer cs-fix

And you must GPG-sign your work, certifying that you either wrote the work or otherwise have the right to pass it on to an open-source project. See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#gpg-signing-old-commits.

@omerimzali
Copy link
Contributor Author

@datamweb Allright I'll try these and update the PR.

@datamweb
Copy link
Collaborator

datamweb commented Sep 30, 2023

Allright I'll try these and update the PR.

Sorry, Ok.

Edit:

what do you mean?

@omerimzali omerimzali force-pushed the feature/remove-supportOldDangerousPassword branch from e0d2c5c to 49a8247 Compare September 30, 2023 13:50
@omerimzali
Copy link
Contributor Author

@datamweb I repushed the PR with sign. It was not easy to sign last 3 commits I'm not sure if it went well. and with change of this line below like you mentioned. I use composer-fix but here's the thing, I don't really know why did you ask to change this phpstan config.

1d9a659
'count' => 8,

Feel free to reject the PR I can create a fresh one if it's not okay.

@datamweb
Copy link
Collaborator

I use composer-fix but here's the thing, I don't really know why did you ask to change this phpstan config.

The reason for this request was the failure in examination phpstan. In fact, we had already suppressed the number of 9 errors detected by PHPStan.

With this PR you have removed the following code:

    public function testCheckSuccessOldDangerousPassword(): void
    {
        /** @var Auth $config */
        $config                              = config('Auth');
        $config->supportOldDangerousPassword = true; // @phpstan-ignore-line

        fake(
            UserIdentityModel::class,
            [
                'user_id' => $this->user->id,
                'type'    => Session::ID_TYPE_EMAIL_PASSWORD,
                'secret'  => 'foo@example.com',
                'secret2' => '$2y$10$WswjNNcR24cJvsXvBc5TveVVVQ9/EYC0eq.Ad9e/2cVnmeSEYBOEm',
            ]
        );

        $result = $this->auth->check([
            'email'    => 'foo@example.com',
            'password' => 'passw0rd!',
        ]);

        $this->assertInstanceOf(Result::class, $result); // This causes an error
        $this->assertTrue($result->isOK());

        $foundUser = $result->extraInfo();
        $this->assertSame($this->user->id, $foundUser->id);
    }

Therefore, one error will be less.

Regarding command composer cs-fix, this command automatically checks the code styles and fixes it if there is a problem.

As you can see now, everything turned green:

Screenshot 2023-09-30 194532

@datamweb
Copy link
Collaborator

I repushed the PR with sign. It was not easy to sign last 3 commits I'm not sure if it went well.

Please refer to the link below:

#672 (comment)
#672 (comment)

@omerimzali omerimzali force-pushed the feature/remove-supportOldDangerousPassword branch 4 times, most recently from dbb448b to 652d639 Compare September 30, 2023 17:43
@omerimzali omerimzali force-pushed the feature/remove-supportOldDangerousPassword branch from 652d639 to 157f21f Compare September 30, 2023 17:47
@omerimzali
Copy link
Contributor Author

@datamweb Thank you for all these explanations. Specially on PHPStan part. I was confused at first about replacing 9 to 8.

No I think ı signed well. It seems legit.
image

@datamweb datamweb removed the GPG-Signing needed Pull requests that need GPG-Signing label Sep 30, 2023
@omerimzali
Copy link
Contributor Author

@datamweb 🙏 PR need a reviewer and approving review by reviewers, but will you wait to merge because of the scope of this PR?

@datamweb
Copy link
Collaborator

@omerimzali I added a ref link to your PR description, but haven't had time to review it carefully yet.
Changes may be needed, in any case you will be notified.

@kenjis, what's the next version?

@kenjis
Copy link
Member

kenjis commented Sep 30, 2023

The next version is 1.0.0-beta.7.

@omerimzali
Copy link
Contributor Author

@datamweb thank you, feel free to ping me for any changes.

@kenjis kenjis closed this Dec 3, 2023
@kenjis kenjis reopened this Dec 3, 2023
@kenjis kenjis added the refactor Pull requests that refactor code label Dec 3, 2023
@kenjis
Copy link
Member

kenjis commented Dec 3, 2023

@omerimzali Now some GitHub Acton checks are failed. Can you rebase?
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#updating-your-branch

We are going to release v1.0.0 soon.

@kenjis
Copy link
Member

kenjis commented Dec 8, 2023

No feedback.
Closed by #976

@kenjis kenjis closed this Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Pull requests that refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dev: remove supportOldDangerousPassword before v1.0.0 release
3 participants