Problem/Motivation

Marked for removal in #2753681: Move CSRF header token out of REST module so that user module can use it, as well as any contrib module but never removed.

Steps to reproduce

n/a

Proposed resolution

Remove code.

Remaining tasks

Review.

User interface changes

None.

API changes

None not previously handled in original deprecation.

Data model changes

None.

Release notes snippet

n/a (?)

Issue fork drupal-3343031

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

bradjones1 created an issue. See original summary.

bradjones1’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

Seems there were a number of failures.

bradjones1’s picture

Status: Needs work » Needs review

The login controller was still using the old token. I need help determining if this basically re-starts the clock on actually removing this b/c in theory you could break a login that is happening _during_ a site update? That doesn't particularly seem like a strong case to me because lots of changes can mean a really well-timed request might break during an update, if the site is not in maintenance mode.

bradjones1’s picture

The discussion around this was at #2753681-33: Move CSRF header token out of REST module so that user module can use it, as well as any contrib module however it seems that without updating the login controller the cut-over didn't ever really happen as intended.

Does this mean that we can only change the token used in the login controller now and have to wait for another two minor releases to come out, so the original is out of support?

smustgrave’s picture

Status: Needs review » Needs work

From what I can tell (sorry if I'm off) but this token is still in use or potentially in use? So think we would have to deprecate it to be removed in D11.

bradjones1’s picture

From what I can tell (sorry if I'm off) but this token is still in use or potentially in use? So think we would have to deprecate it to be removed in D11.

This is the sticky wicket. The initial change would have properly deprecated this if the token were changed at the same time as the BC layer were added. Except, the BC layer was added and the use of the deprecated token did not.

So if we were to use the same deprecation procedure, then the BC layer stays, the token changes, and then in two minor releases (when the older one goes out of support) then the BC layer can be removed.

There's part of me that feels like that's a bit much, but it's the only real way to do this to be completely by the book. It would be nice to get some sort of official ruling on this by a framework maintainer.

smustgrave’s picture

Would agree with that.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.