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.

Sign up for GitHub

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

Jump to bottom

BackendUserHandler breaks workspace #535

Open
DanielSiepmann opened this issue Jan 25, 2024 · 7 comments · May be fixed by #536
Open

BackendUserHandler breaks workspace #535

DanielSiepmann opened this issue Jan 25, 2024 · 7 comments · May be fixed by #536

Comments

@DanielSiepmann
Copy link
Contributor

DanielSiepmann commented Jan 25, 2024 •
edited
Loading

What are you trying to achieve?

A frontend request with enabled backend user.
As we extend admin panel and want to cover that with functional tests executing frontend requests.

What do you get instead?

We receive a 404 due to BackendUserHandler class which breaks the expected workspace aspect.

How to reproduce the issue?

Execute the following test:

        $request = new InternalRequest();
        $request = $request->withPageId(1);
        $request = $request->withLanguageId($languageUid);
        $requestContext = new InternalRequestContext();
        $requestContext = $requestContext->withBackendUserId(1);
        $html = (string) $this->executeFrontendSubRequest($request, $requestContext)->getBody();

With the following database fixture:

<?php

declare(strict_types=1);

return [
    'pages' => [
        [
            'uid' => 1,
            'pid' => 0,
            'title' => 'Page with uid 1 below 0',
            'deleted' => 0,
            'backend_layout' => 'pagets__21',
            'slug' => '/start',
        ],
    ],
    'be_users' => [
        [
            'uid' => 1,
            'pid' => 0,
            'tstamp' => 1366642540,
            'username' => 'editor',
            // The actual password is: password
            'password' => '$1$tCrlLajZ$C0sikFQQ3SWaFAZ1Me0Z/1',
            'admin' => 0,
            'disable' => 0,
            'starttime' => 0,
            'endtime' => 0,
            'options' => 0,
            'crdate' => 1366642540,
            'workspace_perms' => 1,
            'deleted' => 0,
            'TSconfig' => null,
            'lastlogin' => 1371033743,
            'workspace_id' => 0,
            // Necessary to allow user to access this page.
            'db_mountpoints' => 1,
            // Necessary to open admin panel.
            'uc' => serialize([
                'AdminPanel' => [
                    'display_top' => true,
                ],
            ]),
        ],
    ],
];

Additional information you would like to provide?

It looks like https://github.com/TYPO3/testing-framework/blob/8.0.8/Resources/Core/Functional/Extensions/json_response/Classes/Middleware/BackendUserHandler.php doesn't properly set the workspace property until one explicitly provides it within the test. Leading to keeping the -99 fallback which in turn will lead to other follow up issues as the system status is wrong.

That will lead to the expectation the user is in preview mode, which will trigger all kind of access checks for a public page.

Specify some data of the environment

  • TYPO3 testing framework version: 8.0.8
  • TYPO3 version: 12.4.10
  • TYPO3 installation type: composer
  • PHP version: 8.2.14
  • Web server used + version: None
  • Operating system: Ubuntu
@DanielSiepmann
Copy link
Contributor Author

Looks like falling back on 0 if not provided would solve the issue.

DanielSiepmann added a commit to DanielSiepmann/fork-testing-framework that referenced this issue Jan 25, 2024
@DanielSiepmann
[BUGFIX] Do not end up in workspace and preview for default requests
d48c23e 
Default requests won't have a workspace id.
That will lead to TYPO3 defaults -99 id.
That in turn will lead to being in preview mode once a backend user is
provided within context.

We therefore cover the default and turn it into live workspace which
could be the expected default.

Resolves: TYPO3#535
@DanielSiepmann DanielSiepmann linked a pull request Jan 25, 2024 that will close this issue
Open
@DanielSiepmann
Copy link
Contributor Author

DanielSiepmann commented Jan 25, 2024 •
edited
Loading

Looks like the middleware misses some more proper initializations, e.g. UserTSconfig seems to be missing as well.

Looks like those two calls should be added as well:

            $backendUser->initializeUserSessionManager();
            $backendUser->fetchGroupData();

And UC is also not unpacked … And the necessary method is now protected and therefore not available from the middleware.

@sbuerk
Copy link
Collaborator

sbuerk commented Jan 25, 2024

Can you provide a full testcase showing the issue ? (failing) - can be done as a WIP core patch.

TBH - not getting how parts are looking in the test setup not mentioned in the post/thread.

@DanielSiepmann
Copy link
Contributor Author

Sure, but will take some time as I moved forward and work on other areas right now …

DanielSiepmann added a commit to DanielSiepmann/fork-testing-framework that referenced this issue Jan 30, 2024
@DanielSiepmann
[BUGFIX] Do not end up in workspace and preview for default requests
36a137f 
Default requests won't have a workspace id.
That will lead to TYPO3 defaults -99 id.
That in turn will lead to being in preview mode once a backend user is
provided within context.

We therefore cover the default and turn it into live workspace which
could be the expected default.

Resolves: TYPO3#535
@DanielSiepmann
Copy link
Contributor Author

@sbuerk done: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82715 You can debug within typo3/sysext/adminpanel/Classes/Utility/StateUtility.php to see that the TSconfig is not provided, due to missing intialization. This is called from e.g. typo3/sysext/adminpanel/Classes/Middleware/AdminPanelInitiator.php.

The provided PR fixes most of the issues for 12.4 and main.
But I couldn't find a way to unpack the uc of the user. That prevents to "open" the admin panel, the 2nd test.

@DanielSiepmann
Copy link
Contributor Author

Another proper solution might be to replace the provided middleware of testing framework with an authenticator which checks the provided request context and auths the user, allowing TYPO3 to work as usual? But I didn't give it a try yet.

DanielSiepmann added a commit to DanielSiepmann/fork-testing-framework that referenced this issue Feb 12, 2024
@DanielSiepmann
[BUGFIX] Properly initialize backend user for requests
3297eff 
Default requests won't have a workspace id.
That will lead to TYPO3 defaults -99 id.
That in turn will lead to being in preview mode once a backend user is
provided within context.

We therefore cover the default and turn it into live workspace which
could be the expected default.

Also we add missing initialization for backend user.

Resolves: TYPO3#535
@DanielSiepmann
Copy link
Contributor Author

The given PR solves the issues and makes https://review.typo3.org/c/Packages/TYPO3.CMS/+/82715 pass.

DanielSiepmann added a commit to DanielSiepmann/fork-testing-framework that referenced this issue Feb 21, 2024
@DanielSiepmann
[BUGFIX] Properly initialize backend user for requests
c1cf194 
Default requests won't have a workspace id.
That will lead to TYPO3 defaults -99 id.
That in turn will lead to being in preview mode once a backend user is
provided within context.

We therefore cover the default and turn it into live workspace which
could be the expected default.

Also we add missing initialization for backend user.

Resolves: TYPO3#535
sbuerk pushed a commit to DanielSiepmann/fork-testing-framework that referenced this issue Nov 29, 2024
@DanielSiepmann @sbuerk
[BUGFIX] Properly initialize backend user for requests
eb72b0a 
Default requests won't have a workspace id.
That will lead to TYPO3 defaults -99 id.
That in turn will lead to being in preview mode once a backend user is
provided within context.

We therefore cover the default and turn it into live workspace which
could be the expected default.

Also we add missing initialization for backend user.

Resolves: TYPO3#535
sbuerk added a commit to web-vision/deepltranslate-core that referenced this issue Dec 14, 2024
@sbuerk
[TASK] Add regression test for issue #395
f7ef778 
Issue #395 has been recently fixed for 4.x and 5.x
with following pull-requests:

* #397
* #398

Adding regression test has been omited due to issues within the
`typo3/testing-framework` already. The major missing part boils
down to an incomplete frontend backend user authentication setup,
reported by Daniel Siepmann [1] and provided a pull-request [2].

* [1] Issue: TYPO3/testing-framework#535
* [2] Pull-request: TYPO3/testing-framework#536

The pull-request has been adopted by providing a test-fixture
extension replacing the BackendUserHandler with a fixed version.
Sadly, this required an additional temporary workaround within
the functional test bootstrap, enforce loading package infor for
the testing-framework extensions. That needs to be fixed within
`typo3/testing-framework`, enforce loading the package info in
Testbase->linkFrameworkExtensionsToInstance` when linking the
framework extension - or refactor the linking all together.

For ongoing work to add TYPO3 v13 support we take the workarounds
to continue the work having regression test in place and do not
wait until resolved in the testing-framework which needs adoptions
in the TYPO3 monorepo and therefore some coordination in fix, release
and upgrade.

SiteBasedTestTrait related reporting for new test case are added
to the baseline for now, needs to be addressed in general with
a dedcated change.

Used command(s):

```bash
Build/Scripts/runTests.sh -t 12 -p 8.1 -s phpstanGenerateBaseline
```

Related: #395
Related: #398
Related: #397
@sbuerk sbuerk mentioned this issue Dec 14, 2024
Merged
sbuerk added a commit to web-vision/deepltranslate-core that referenced this issue Dec 14, 2024
@sbuerk
[TASK] Add regression test for issue #395
dd81d1b 
Issue #395 has been recently fixed for 4.x and 5.x
with following pull-requests:

* #397
* #398

Adding regression test has been omited due to issues within the
`typo3/testing-framework` already. The major missing part boils
down to an incomplete frontend backend user authentication setup,
reported by Daniel Siepmann [1] and provided a pull-request [2].

* [1] Issue: TYPO3/testing-framework#535
* [2] Pull-request: TYPO3/testing-framework#536

The pull-request has been adopted by providing a test-fixture
extension replacing the BackendUserHandler with a fixed version.
Sadly, this required an additional temporary workaround within
the functional test bootstrap, enforce loading package infor for
the testing-framework extensions. That needs to be fixed within
`typo3/testing-framework`, enforce loading the package info in
Testbase->linkFrameworkExtensionsToInstance` when linking the
framework extension - or refactor the linking all together.

For ongoing work to add TYPO3 v13 support we take the workarounds
to continue the work having regression test in place and do not
wait until resolved in the testing-framework which needs adoptions
in the TYPO3 monorepo and therefore some coordination in fix, release
and upgrade.

SiteBasedTestTrait related reporting for new test case are added
to the baseline for now, needs to be addressed in general with
a dedcated change.

Used command(s):

```bash
Build/Scripts/runTests.sh -t 12 -p 8.1 -s phpstanGenerateBaseline
```

Related: #395
Related: #398
Related: #397
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@DanielSiepmann @sbuerk