Views argument validators aren't taken into account on access checks - local task point to 404

Created on 3 August 2016, over 8 years ago
Updated 18 December 2023, about 1 year ago

Problem/Motivation

Steps to reproduce:

- setup a view with a path like node/%node/foo
- use a conditional argument
- add validation on node type to the conditioanl argument
- add a local task entry for the view via the views UI

Expected result: the local task doesn't show up for invalid node types
Actual result: it does

Proposed resolution

Somehow translate the argument validation to access checking - we'd want to keep the behaviour of not loading the view though, and might not be possible to do this generically - i.e. we might have to create access checks 1-1 with validator plugins?

Remaining tasks

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

Needs review

Version

11.0 πŸ”₯

Component
ViewsΒ  β†’

Last updated 11 minutes ago

Created by

πŸ‡¬πŸ‡§United Kingdom catch

Live updates comments and jobs are added and updated live.
  • Triaged core major

    There is consensus among core maintainers that this is a major issue. Only core committers should add this tag.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡§πŸ‡ͺBelgium Jonasanne

    Reroll patch #46 for 10.2.x

  • last update about 1 year ago
    Custom Commands Failed
  • Status changed to Needs work about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Also recommend converting to MRs as easier reviews.

    Still needs sub-maintainer review but NW for the CC failures.

  • First commit to issue fork.
  • Pipeline finished with Failed
    8 months ago
    Total: 177s
    #149677
  • Pipeline finished with Failed
    8 months ago
    Total: 184s
    #149685
  • Pipeline finished with Canceled
    8 months ago
    Total: 91s
    #149692
  • Status changed to Needs review 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States dww

    I just ran into this bug on a new project. I was amazed this was broken. I dug deep into the views code, only to find:

    // @todo How do we apply argument validation?
    

    buried in core/modules/views/src/Plugin/views/display/PathPluginBase.php.

    Searched the d.o queue and landed here. Thankfully, patch #52 applies cleanly both to 11.x HEAD and my 10.2.5 site. Converted to an MR. Fixed the phpcs and phpstan errors. I haven't addressed #35 / #47, but wanted to get this back into a reviewable state.

    Let's smash this bug!

    Thanks,
    -Derek

  • πŸ‡ΊπŸ‡ΈUnited States dww

    Hiding all attached files and starting to save credits.

  • Pipeline finished with Failed
    8 months ago
    #149693
  • Pipeline finished with Failed
    8 months ago
    Total: 1079s
    #149698
  • Status changed to Needs work 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States dww

    Well, drat. There are legit test failures in both of these:

    core/modules/system/tests/src/Functional/Menu/BreadcrumbTest.php
    core/modules/taxonomy/tests/src/Functional/TermTest.php
    

    I'm out of time for today, but I hope to get back to this soon. πŸ˜… If anyone else wants to run with it in the meanwhile, please do. πŸ™

    Thanks,
    -Derek

  • πŸ‡¬πŸ‡§United Kingdom jonathanshaw Stroud, UK

    @dww you may be encountering #35 / #47

  • πŸ‡¬πŸ‡§United Kingdom rachel_norfolk UK

    Just tidying tags

Production build 0.71.5 2024