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

Allow anonymous mode for AdminController #15855

Merged
merged 2 commits into from Oct 9, 2019

Conversation

@PierreRambaud
Copy link
Contributor

PierreRambaud commented Oct 7, 2019

Questions Answers
Branch? 1.7.6.x
Description? Add allowAnonymous property in AdminController in order to execute cron.
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #15725
How to test? The cron script can be executable without having an employee connected.

Example:

    /**
     * {@inheritdoc}
     */
    public function init()
    {
        if ($this->isCronTask()
            && substr(
                _COOKIE_KEY_,
                static::TOKEN_CHECK_START_POS,
                static::TOKEN_CHECK_LENGTH
            ) === Tools::getValue('token')
        ) {
            $this->isAnonymousAllowed(true);
        }

        parent::init();
    }

    /**
     * Check if a task is a cron task
     *
     * @return bool
     */
    protected function isCronTask()
    {
        return Tools::isSubmit('action') && 'searchCron' === Tools::getValue('action');
    }

This change is Reviewable

@PierreRambaud PierreRambaud added this to the 1.7.6.2 milestone Oct 7, 2019
@PierreRambaud PierreRambaud requested a review from PrestaShop/prestashop-core-developers as a code owner Oct 7, 2019
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Oct 7, 2019

@PierreRambaud So security is 100% provided by the token ?

@PierreRambaud PierreRambaud force-pushed the PierreRambaud:fix/15725 branch from 4f68d4e to 4ec8c90 Oct 7, 2019
@PierreRambaud

This comment has been minimized.

Copy link
Contributor Author

PierreRambaud commented Oct 7, 2019

@matks Yes it is, and only when running in anonymous mode. But the token is not the same as the one used for csrf protection.

@PierreRambaud PierreRambaud force-pushed the PierreRambaud:fix/15725 branch 3 times, most recently from c2e0c5a to df24db4 Oct 7, 2019
@sarahdib sarahdib self-assigned this Oct 8, 2019
@PierreRambaud PierreRambaud force-pushed the PierreRambaud:fix/15725 branch from df24db4 to 4fe82f7 Oct 8, 2019
@sarahdib sarahdib added QA ✔️ and removed waiting for QA labels Oct 9, 2019
@matks
matks approved these changes Oct 9, 2019
@matks matks merged commit 42d4834 into PrestaShop:1.7.6.x Oct 9, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Oct 9, 2019

Thank you @PierreRambaud

@PierreRambaud PierreRambaud deleted the PierreRambaud:fix/15725 branch Oct 9, 2019
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Oct 31, 2019

There might a BC break in this PR 🤔see the diff

@eternoendless eternoendless added BC break and removed BC break labels Oct 31, 2019
@eternoendless

This comment has been minimized.

Copy link
Member

eternoendless commented Oct 31, 2019

There's no breaking change actually, the removed function exists in the parent class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.