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

Can't submit modules in DTGE after Umbraco patch if user has Start Nodes configured #15460

Closed
lukehook opened this issue Dec 14, 2023 · 20 comments

Comments

@lukehook
Copy link

lukehook commented Dec 14, 2023

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

8.18.11

Bug summary

On an existing v8 site using DocTypeGridEditor, users cannot submit a module / doctype in a grid editor.

When an attempt is made to do so the following error is displayed

image

This occurs on ANY user account that has start nodes configures and appears to be to do with access to Content Templates. If content start nodes are removed the issue is resolved.

Specifics

Umbraco v8.18.11
DocTypeGridEditor installed and a module configured
A user is configured to have custom start nodes

Steps to reproduce

  • Create a content DocType and configure a DocTypeGridEditor with a module for that DocType
  • Create Content item in the tree
  • Set user Content Start Node to this content item (or a parent there of)
  • Attempt to add an instance of the module to the DTGE
  • You should be presented with the error presented in the screenshot

Expected result / actual result

User should be able to submit content without issue

@skttl - I'm fully aware DTGE has long since been retired and I appreciate this bug may be more associated with that package directly (and therefore likely won't get fixed) but I felt it necessary to flag this given the error pertains to permissions for Content Blueprints as it may uncover an unintended bug with the patch itself given it only appeared since upgrading 🙏

Copy link

Hi there @lukehook!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

  • We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
  • If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
  • We'll replicate the issue to ensure that the problem is as described.
  • We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@Zeegaan
Copy link
Member

Zeegaan commented Dec 15, 2023

Hmm I don't know how DTGE works, but it does sound like an Umbraco regression, maybe @skttl could help uncover the issue 🙈

@Zeegaan Zeegaan added the state/needs-more-info We don't have enough information to give a good reply label Dec 15, 2023
@skttl
Copy link
Contributor

skttl commented Dec 15, 2023

DTGE saves a temporary Content Template of all grid editors, so the editor must have permissions to create those.

@SteveVaneeckhout
Copy link
Contributor

SteveVaneeckhout commented Dec 15, 2023

And it seems that users with Content Start Nodes set other than root are unable to create Content Templates anymore.

EDIT: Did some more digging:
A user with limited Content Start Nodes can create a new Content Template when he is in the Content section of Umbraco but not in the Settings section.

@Zeegaan
Copy link
Member

Zeegaan commented Dec 15, 2023

Aha! I think I see the issue then, there is definitely something going wrong here, and I can certainly reproduce it.. 😿

@eks-cthorsoe
Copy link

@bergmania @Zeegaan I see this issue has been marked as closed, but I still get the Unauthorized access to URL when calling the PostSaveBlueprint endpoint from a solution running on version 10.8.3. Is the user not supposed to have access to this if the usergroup has checkmark in Create Content Template?

@bergmania
Copy link
Member

To call PostSaveBlueprint it is required you have access to settings.
There is another endpoint CreateBlueprintFromContent which you have access to, when having checkmark in Create Content Template, even that you do not have access to settings

@lukehook
Copy link
Author

lukehook commented Jan 2, 2024

Just to be clear on this specific issue then @bergmania @skttl, in the instance of DTGE I assume this fix won't solve our problem given it seems to use the PostSaveBlueprint endpoint? (Not a problem of course as I appreciate that's deprecated and NOT umbraco but just so I can manage expectations)

@mcl-sz
Copy link
Contributor

mcl-sz commented Jan 2, 2024

For the same reason I have not upgraded Umbraco 8 projects yet. DTGE is a frequently used component in our sites. @skttl do you see opportunities to release another version that is compatible with Umbraco 8.18.12?

@skttl
Copy link
Contributor

skttl commented Jan 2, 2024

If you provide a pull request, I can do a release.

@Emetico
Copy link

Emetico commented Jan 9, 2024

Hi @skttl

Is the PostSaveBlueprint only called to trigger a validation?

In that case we could replace the submit function in Our.Umbraco.DocTypeGridEditor.Dialogs.DocTypeGridEditorDialog controller

from

function submit() {
    if ($scope.model.submit) {
        serverValidationManager.reset();
        vm.saveButtonState = "busy";
        $scope.model.node.name = "Dtge Temp: " + $scope.model.node.key;
        $scope.model.node.variants[0].name = $scope.model.node.name
        $scope.model.node.variants[0].save = true;

        // Reset route create to prevent showing up the changed content dialog
        var routeParamsCreate = $routeParams.create;
        $routeParams.create = undefined;

        // save the content as a blueprint, to trigger validation
        var args = {
            saveMethod: contentResource.saveBlueprint,
            scope: $scope,
            content: $scope.model.node,
            create: true,
            action: "save",
            showNotifications: false,
            softRedirect: true
        }

        contentEditingHelper.contentEditorPerformSave(args).then(function (data) {
            $scope.model.node.id = data.id;
            $scope.model.submit($scope.model);

            // Reset original value of $routeParams.create
            $routeParams.create = routeParamsCreate;
        },
            function (err) {
                // Set original value of $routeParams.create
                $routeParams.create = routeParamsCreate;
                // cleanup the blueprint immediately
                if (err && err.data) {
                    $scope.model.node.id = err.data.id;
                }
                cleanup();
                vm.saveButtonState = "error";
            });

        vm.saveButtonState = "init";
    }
}

To

function submit() {
    if ($scope.model.submit) {
        serverValidationManager.reset();
        vm.saveButtonState = "busy";
        $scope.model.node.name = "Dtge Temp: " + $scope.model.node.key;
        $scope.model.node.variants[0].name = $scope.model.node.name
        $scope.model.node.variants[0].save = true;

        // Reset route create to prevent showing up the changed content dialog
        var routeParamsCreate = $routeParams.create;
        $routeParams.create = undefined;

        var formSubmitOptions = {
            scope: $scope,
            action: "save"
        };

        if (formHelper.submitForm(formSubmitOptions)) {
            formHelper.resetForm({ scope: $scope });

            $scope.model.submit($scope.model);
        }
        else {
            cleanup();
            vm.saveButtonState = "error";
        }

        // Reset original value of $routeParams.create
        $routeParams.create = routeParamsCreate;

        vm.saveButtonState = "init";
    }
}

@skttl
Copy link
Contributor

skttl commented Jan 9, 2024

Yes, but then you won't have validation.

@mcl-sz
Copy link
Contributor

mcl-sz commented Jan 9, 2024

That is indeed the reason why PostSaveBlueprint is used, especially for nested content. I looked into making my own controller that could only do validation of a content item, but this turned out to be anything but simple. All validation in Umbraco is in private classes and is therefore not easy to implement

@Emetico
Copy link

Emetico commented Jan 9, 2024

I see.

We are talking about the serverside validation and not the form validation ofcause.

@Emetico
Copy link

Emetico commented Jan 10, 2024

That is indeed the reason why PostSaveBlueprint is used, especially for nested content. I looked into making my own controller that could only do validation of a content item, but this turned out to be anything but simple. All validation in Umbraco is in private classes and is therefore not easy to implement

I made a copy of the ContentController and called it DocTypeGridEditorContentController and then copied all of the required internal references and removed all unnessary code that is not being used by the PostSaveBlueprint and DeleteBlueprint in the ContentController. Then i removed the [Authorize(Policy = AuthorizationPolicies.TreeAccessDocuments)] and replaced it with [Authorize(Policy = AuthorizationPolicies.SectionAccessForContentTree)], so all users who have access to the Content can use it.

This works with 10.8.3. But leaves an unsafe route to call a SaveBlueprint , when you are logged in and not having access to the Settings section.

It seems like all the validation is made before and after the SaveBlueprint call.
So i was wondering if the actual SaveBlueprint call inside the PostSaveBlueprint method is required at all for the validation?

@mcl-sz
Copy link
Contributor

mcl-sz commented Jan 10, 2024

It seems like all the validation is made before and after the SaveBlueprint call. So i was wondering if the actual SaveBlueprint call inside the PostSaveBlueprint method is required at all for the validation?

As far as i know it's only used for validation and the save-part is not necessary. I tried the same for Umbraco 8, but it requires a lot of copying of existing classes. Personally, I don't think that is a nice solution.

@Emetico
Copy link

Emetico commented Jan 10, 2024

It seems like all the validation is made before and after the SaveBlueprint call. So i was wondering if the actual SaveBlueprint call inside the PostSaveBlueprint method is required at all for the validation?

As far as i know it's only used for validation and the save-part is not necessary. I tried the same for Umbraco 8, but it requires a lot of copying of existing classes. Personally, I don't think that is a nice solution.

I totally agree that it's not a nice solution and not something i think DTGE should implement.
But it is sadly the only solution we can come up with at the moment, so we can upgrade to the newest security versions.

I think that the best solution would be for Umbraco CMS to add a public BlueprintValidation method that DTGE could use.

@mcl-sz
Copy link
Contributor

mcl-sz commented Jan 10, 2024

I think that the best solution would be for Umbraco CMS to add a public BlueprintValidation method that DTGE could use.

That would indeed be a nice solution, which also only needs to consist of less than 10 lines of code.

I have created a feature request #15560

@si25
Copy link

si25 commented Jan 29, 2024

Hi @Emetico @mcl-sz ,

Don't suppose either of you came up with a solution for DTGE on Umbraco 8.18.12? Running into the same issue with non-admin users

Thanks,
Simon

@skttl
Copy link
Contributor

skttl commented Jan 29, 2024

@si25 a fix might be coming for DTGE: #15572 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants