More global and superglobal cleanup

Created on 21 January 2023, over 1 year ago
Updated 9 February 2023, over 1 year ago

Problem/Motivation

I noticed when I was merging ๐Ÿ“Œ Massive code cleanup Fixed that there are still references to PHP superglobals ($_GET, $_POST, &c.), and those have been deprecated since Drupal 8. โ†’

Also, saml_sp_start() was getting the current language and global $base_url but never using either.

Proposed resolution

Attached MR removes the remaining superglobals from all the uncommented code, as well as the two spurious variables from saml_sp_start().

Remaining tasks

There are both globals and superglobals in the commented-out saml_sp_user_logout. The patch in ๐Ÿ“Œ Enabled hook_user_logout functionality Needs work removes them, so when that gets rerolled and merged those will be handled.

There are also superglobals in deprecated saml_sp__is_valid_authentication_response(); I decided not to change them since it will be gone soon enough.

API changes

SamlSPController now uses dependency injection to get the current request. Its private validAuthenticationResponse() method now expects that object as a parameter, and logs a warning if it doesnโ€™t get one.

(I considered doing the same for SamlSPDrupalLoginController to pass to saml_sp_start(), but it felt like overkill. Why isnโ€™t the request just readily available in all controllers, anyway?)

๐Ÿ“Œ Task
Status

Fixed

Version

4.0

Component

Code

Created by

๐Ÿ‡บ๐Ÿ‡ธUnited States jproctor

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

Comments & Activities

Production build 0.69.0 2024