Skip to content

Conversation

@dereuromark
Copy link
Member

Prepare #716

@dereuromark dereuromark closed this Nov 6, 2025
@dereuromark dereuromark reopened this Nov 6, 2025
@dereuromark
Copy link
Member Author

For some reason 4.x is now protected already, even though just created.
I have the ready branch at "4.x-squashed", maybe someone with access can just force push into 4.x?

@dereuromark dereuromark requested a review from ADmad November 6, 2025 06:04
@ADmad
Copy link
Member

ADmad commented Nov 6, 2025

For some reason 4.x is now protected already, even though just created.

The protection rule is set for *.x, that's way. I have temporarily changed it to 3.x, so you should be able to force push to 4.x now.

@dereuromark dereuromark marked this pull request as ready for review November 6, 2025 14:27
@dereuromark
Copy link
Member Author

Auth code looks pretty clean and cake-user-friendly now, doesnt it?

@dereuromark dereuromark changed the title Remove deprecations, fix up MultiChecker. 4.x major: Remove deprecations, fix up MultiChecker. Nov 7, 2025
@dereuromark dereuromark added this to the 4.x milestone Nov 20, 2025
@dereuromark
Copy link
Member Author

@ADmad How do we merge our regression fix into this?
Do we want to keep the "cleaner" constructor of 4.x? Or do we move to the lazy defaulting?

@ADmad
Copy link
Member

ADmad commented Nov 29, 2025

Doing it in getIdentifier() does have the benefit that the objects chain gets initialized only when needed.

- Use lazy identifier initialization in getIdentifier() (3.x approach)
- Add AuthenticationPlugin as main plugin class, Plugin as deprecated alias
- Add redirect validation feature from 3.x
- Update all authenticators to use getIdentifier() instead of direct property access
Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. I think the impacts of these breaking changes should be a reasonable amount of work in a small section for most applications. I don't think we need to worry about rector here. I like how the authenticator + identifier relationship has ended up working.

@dereuromark
Copy link
Member Author

Just to be clear:

Config needs to change from:

  $identifier = [
      'Authentication.Token' => [
          'tokenField' => 'id',
          'dataField' => 'key',
          'resolver' => [
              'className' => 'Authentication.Orm',
              'finder' => 'auth',
          ],
      ],
  ];

To:

  $identifier = [
      'className' => 'Authentication.Token',
      'tokenField' => 'id',
      'dataField' => 'key',
      'resolver' => [
          'className' => 'Authentication.Orm',
          'finder' => 'auth',
      ],
  ];

We could shim this (allow for the old array one), but then it would be possible to also insert multiple array key/value pairs or alike.

@ADmad
Copy link
Member

ADmad commented Dec 12, 2025

I think just clarifying the change in a migration guide should suffice

dereuromark added a commit to cakephp/upgrade that referenced this pull request Dec 13, 2025
Adds rules to automate the 3.x to 4.x migration for the authentication plugin:

- Rename CakeRouterUrlChecker to DefaultUrlChecker
- Rename DefaultUrlChecker (framework-agnostic) to GenericUrlChecker
- Rename Plugin to AuthenticationPlugin
- Remove loadIdentifier() method calls

See: cakephp/authentication#748
@dereuromark
Copy link
Member Author

Some things are auto-upgradable: cakephp/upgrade#370

Shall we start the release process on this major?

@ADmad
Copy link
Member

ADmad commented Dec 13, 2025

Shall we start the release process on this major?

Since we are doing a new major I think we can make this a "proper" CakePHP plugin, i.e. add cakephp/cakephp to composer's require config. I highly doubt anyone is using the auth plugin outside of Cake.

If we do so then some housekeeping that can be done is, rename GenericUrlChecker to StringUrlChecker (and update it's docblock description). Similarly change/cleanup any other code which was meant for framework agnostic usage (can't think of anything off the top of my head though).

@dereuromark
Copy link
Member Author

Technically, https://github.com/cakephp/authorization/blob/459cd4d752b3b93ff49791b33ae42a04b82cceb4/composer.json#L27 also only has one package

Couldnt we do the housekeeping for it anyways? and document it as such?

@ADmad
Copy link
Member

ADmad commented Dec 13, 2025

Yes we can do the same for Authorization too.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants