Allow if no params in URL or none in configuration. Add test coverage as well.

Created on 1 December 2023, about 1 year ago
Updated 29 July 2024, 5 months ago

Problem/Motivation

I set an allowed route to /user/ and didn't set any URL parameters. Once logged in, it gave me a 403 error because the url of the page displayed is /user/42?check_logged_in=1 (...with a query string).

Steps to reproduce

  1. Install with composer and enable version 2.0.0-beta1 on Drupal 10
  2. Add a new restricted route with these parameters
    • Route name or path: /user/
    • Request methods: GET, POST
    • URL parameters: Leave it empty
    • Ips: Enter your ip
    • Operation: Allow access
    • Check Enabled
  3. In your browser, open a new private page and go to /user/login of your site. You should see it.
  4. Log in
  5. You should have a 403 error if there's a quey string in the url.
    • Otherwise, go to /admin/people and edit any user. The url should be /user/%/edit?destination=/admin/people and you should have a 403 error.

Proposed resolution

Logically, if I don't set any URL parameters, it should accept any query string passed to the URL. And looking at the code I think that's what it was supposed to do, but a logic error broke the condition.

I believe the problem comes from the condition present on line 256 of RestrictIpService.php : if (!$params && !$query_string) {

In fact, it could be if (!$params || ($params && !$query_string)) { to check if URL parameters is not set, and otherwise if it's the case, to check if a query string is not passed to the page.

In addition, none of this code has test coverage, so that should be added.

Remaining tasks

Review the submitted merge request.

🐛 Bug report
Status

Needs review

Version

2.0

Component

Code

Created by

🇨🇦Canada franckylfs Montreal

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Production build 0.71.5 2024