Skip to content

IBX-8356: Reworked Ibexa\Core\MVC\Symfony\Security\Authentication\AuthenticatorInterface usages to comply with Symfony-based authentication#101

Merged
konradoboza merged 37 commits intomainfrom
ibx-8356-removed-authenticatorinterface
Jun 25, 2024
Merged

IBX-8356: Reworked Ibexa\Core\MVC\Symfony\Security\Authentication\AuthenticatorInterface usages to comply with Symfony-based authentication#101
konradoboza merged 37 commits intomainfrom
ibx-8356-removed-authenticatorinterface

Conversation

@konradoboza
Copy link
Copy Markdown
Contributor

@konradoboza konradoboza commented Jun 5, 2024

🎫 Issue IBX-8356

Related PRs:

Description:

Reimplemented JWT authentication. The need for that comes from the fact, that Ibexa\Core\MVC\Symfony\Security\Authentication\AuthenticatorInterface relies on deprecated Symfony authorization mechanisms and will be dropped.

Reworked to be compliant with https://symfony.com/bundles/LexikJWTAuthenticationBundle/current/index.html#symfony-5-3-and-higher. The third-party provider has already moved to Symfony json_login authenticator so we are just adapting our implementation accordingly.

Few important changes:

  • XML input is not supported from now on. Only Content-Type: application/vnd.ibexa.api.JWT+json header can still be used for keeping the whole solution BC safe, however the new authenticator should be referenced via Content-Type: application/json header instead,
  • to make sure this is possible I added JsonLoginHeaderReplacingSubscriber which replaces mentioned header on the fly. I also left a TODO note to remember it should be dropped once the new REST API version is released. I am open for suggestions how to improve such reminder,
  • I also removed SecurityListener and kind of replaced it with AuthenticationSuccessSubscriber - it sets current repository user on successful authentication and normalize the end response to be in-tact with the previous one (another BC-safe piece of code). The reason for that is that json_login handles the response outside and forms it similar to:
{
    "token" : "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXUyJ9.eyJleHAiOjE0MzQ3Mjc1MzYsInVzZXJuYW1lIjoia29ybGVvbiIsImlhdCI6IjE0MzQ2NDExMzYifQ.nh0L_wuJy6ZKIQWh6OrW5hdLkviTs1_bau2GqYdDCB0Yqy_RplkFghsuqMpsFls8zKEErdX5TYCOR7muX0aQvQxGQ4mpBkvMDhJ4-pE4ct2obeMTr_s4X8nC00rBYPofrOONUOR4utbzvbd4d2xT_tj4TdR_0tsr91Y7VskCRFnoXAnNT-qQb7ci7HIBTbutb9zVStOFejrb4aLbr7Fl4byeIEYgp2Gd7gY"
}
  • I removed the whole logic from Ibexa\Rest\Server\Controller\JWT::createToken - all the heavy lifting is done by the json_login authenticator so it's not needed anymore. The whole controller was left though, as to my understanding it is needed for the route to be configured properly. If this is wrong, the whole class can just be dropped,
  • I removed classes needed for input parsing and value object visiting since payload/response are formed differently as they become obsolete,
  • src/lib/Security/AuthorizationHeaderRESTRequestMatcher.php was moved out of contracts and marked as @internal as we shouldn't allow manipulating with such vital part of the authentication process,
  • I added test coverage to the introduced event subscribers,
  • I improved the code quality in several classes by resolving outstanding PHPStan issues.

For QA:

Documentation:

We need to mention changes to payload, response and headers described above as much as removed/moved classes.

@konradoboza konradoboza changed the title IBX-8356: Reworked `Ibexa\Core\MVC\Symfony\Security\Authentication\Au… IBX-8356: Reworked Ibexa\Core\MVC\Symfony\Security\Authentication\AuthenticatorInterface usages to comply with Symfony-based authentication Jun 5, 2024
@konradoboza konradoboza force-pushed the ibx-8290-re-implement-rest-auth branch from b0508ca to a3b1471 Compare June 6, 2024 09:24
@konradoboza konradoboza force-pushed the ibx-8356-removed-authenticatorinterface branch 5 times, most recently from d7547bf to e73f707 Compare June 6, 2024 13:13
@konradoboza konradoboza force-pushed the ibx-8290-re-implement-rest-auth branch 2 times, most recently from 9d185ea to 5c75500 Compare June 7, 2024 07:03
@konradoboza konradoboza force-pushed the ibx-8356-removed-authenticatorinterface branch 3 times, most recently from bce578a to 02f4263 Compare June 7, 2024 09:49
@konradoboza konradoboza force-pushed the ibx-8290-re-implement-rest-auth branch from 9c8f8c2 to b23cde0 Compare June 7, 2024 10:52
@konradoboza konradoboza force-pushed the ibx-8356-removed-authenticatorinterface branch from 02f4263 to 37babd1 Compare June 7, 2024 14:17
@konradoboza konradoboza marked this pull request as ready for review June 10, 2024 12:49
@konradoboza konradoboza added Ready for review Doc needed The changes require some documentation labels Jun 10, 2024
@konradoboza konradoboza requested a review from a team June 10, 2024 12:58
Comment thread src/lib/Security/AuthorizationHeaderRESTRequestMatcher.php
Comment thread src/lib/Security/EventListener/JWT/AuthenticationSuccessSubscriber.php Outdated
@konradoboza konradoboza requested review from a team and alongosz June 11, 2024 06:16
@Steveb-p
Copy link
Copy Markdown
Contributor

I would need to know why the matcher specifically is being dropped, and what will be the new expected user configuration.

We used to use Matcher to allow firewalls to detect that they need to perform authorization in a specific way - and I assume that we are going to change it so that authentication mechanisms are no longer tied to firewalls like that. But I need to see the expected end result to judge.

@konradoboza
Copy link
Copy Markdown
Contributor Author

I would need to know why the matcher specifically is being dropped

Can you please elaborate which matcher you have in mind? The configuration is tackled within https://github.com/ibexa/recipes-dev/pull/122/files.

@konradoboza konradoboza force-pushed the ibx-8356-removed-authenticatorinterface branch from 3173e5b to 50f9fbe Compare June 11, 2024 07:50
@Steveb-p
Copy link
Copy Markdown
Contributor

I would need to know why the matcher specifically is being dropped

Can you please elaborate which matcher you have in mind? The configuration is tackled within https://github.com/ibexa/recipes-dev/pull/122/files.

Ibexa\Rest\Security\AuthorizationHeaderRESTRequestMatcher (known before as Ibexa\Contracts\Rest\Security\AuthorizationHeaderRESTRequestMatcher). As @alongosz mentioned in his comments, if it's intended to be used in the configuration as a value, it has to remain in Contracts namespace.

@konradoboza konradoboza force-pushed the ibx-8356-removed-authenticatorinterface branch from 99e5fea to c7df328 Compare June 20, 2024 13:32
@konradoboza konradoboza force-pushed the ibx-8356-removed-authenticatorinterface branch from e94ba89 to 35307f0 Compare June 20, 2024 13:47
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Small suggestion for follow-up PR:

When using unsupported XML format error description could be more user friendly.
Now it states: "You must provide a ValueObject for visiting, "NULL" provided."

@micszo micszo removed their assignment Jun 25, 2024
@konradoboza konradoboza merged commit dedcfa0 into main Jun 25, 2024
@konradoboza konradoboza deleted the ibx-8356-removed-authenticatorinterface branch June 25, 2024 14:18
@Steveb-p
Copy link
Copy Markdown
Contributor

Small suggestion for follow-up PR:

When using unsupported XML format error description could be more user friendly. Now it states: "You must provide a ValueObject for visiting, "NULL" provided."

It's something we might address when swapping over to Symfony Serializer for request parsing. Our current Visitors are barely performing sanity checks at all, and that error sounds more like something PHP would spew on it's own.

adriendupuis added a commit to ibexa/documentation-developer that referenced this pull request Mar 26, 2026
adriendupuis added a commit to ibexa/documentation-developer that referenced this pull request Apr 9, 2026
* development_security.md: Update JWT firewalls

ibexa/recipes-dev#122
ibexa/recipes-dev#124
ibexa/recipes-dev#125

* rest_api_authentication.md: XML isn't supported for JWT

ibexa/rest#101
julitafalcondusza added a commit to ibexa/documentation-developer that referenced this pull request Apr 10, 2026
…ed in Dev-doc (#3127)

* Update JWT (#3108)

* development_security.md: Update JWT firewalls

ibexa/recipes-dev#122
ibexa/recipes-dev#124
ibexa/recipes-dev#125

* rest_api_authentication.md: XML isn't supported for JWT

ibexa/rest#101

* render doc added

* updates

* img added, content added

* mkdocs

* fixes after rev

* fix

* fixes

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>
mnocon pushed a commit to ibexa/documentation-developer that referenced this pull request Apr 14, 2026
* first batch added

* content added

* recommendations twig page added

* updates

* content moved, structure updated

* guide added

* guide content

* fixes, content moved

* fixes after review

* Raptor integration - feature branch: fix links (#3123)

* Add preview of linked PHP API Ref entries
* Fix few links

* new batch of fixes

* updates

* new blocks added

* name fix

* fix

* new page added to cards on raptor_connector landing page

* fixes - first batch

* new fixes

* PHP & JS CS Fixes

* new fixes

* block names fixed

* code fixed

* code fixed

* links fixed

* card fixed

* IBX-11571: Rendering recommendations outside of Page Builder documented in Dev-doc (#3127)

* Update JWT (#3108)

* development_security.md: Update JWT firewalls

ibexa/recipes-dev#122
ibexa/recipes-dev#124
ibexa/recipes-dev#125

* rest_api_authentication.md: XML isn't supported for JWT

ibexa/rest#101

* render doc added

* updates

* img added, content added

* mkdocs

* fixes after rev

* fix

* fixes

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* php fix

* composer.json fix - connector-raptor added

* Raptor integration - feature branch: Fix PHP (#3130)

* EventData.php: Wrap into a class
* Event*.php: Format

* language fixes

* Raptor integration: Rework PHP (#3131)

* tracking_php_api.md: Detail EventMapper, EventType, EventContext
* tracking_php_api.md: caution about buy event

---------

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* server description updated

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>
Co-authored-by: julitafalcondusza <julitafalcondusza@users.noreply.github.com>
Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>
mnocon added a commit to ibexa/documentation-developer that referenced this pull request Apr 20, 2026
* Raptor integration - feature branch (#3121)

* first batch added

* content added

* recommendations twig page added

* updates

* content moved, structure updated

* guide added

* guide content

* fixes, content moved

* fixes after review

* Raptor integration - feature branch: fix links (#3123)

* Add preview of linked PHP API Ref entries
* Fix few links

* new batch of fixes

* updates

* new blocks added

* name fix

* fix

* new page added to cards on raptor_connector landing page

* fixes - first batch

* new fixes

* PHP & JS CS Fixes

* new fixes

* block names fixed

* code fixed

* code fixed

* links fixed

* card fixed

* IBX-11571: Rendering recommendations outside of Page Builder documented in Dev-doc (#3127)

* Update JWT (#3108)

* development_security.md: Update JWT firewalls

ibexa/recipes-dev#122
ibexa/recipes-dev#124
ibexa/recipes-dev#125

* rest_api_authentication.md: XML isn't supported for JWT

ibexa/rest#101

* render doc added

* updates

* img added, content added

* mkdocs

* fixes after rev

* fix

* fixes

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* php fix

* composer.json fix - connector-raptor added

* Raptor integration - feature branch: Fix PHP (#3130)

* EventData.php: Wrap into a class
* Event*.php: Format

* language fixes

* Raptor integration: Rework PHP (#3131)

* tracking_php_api.md: Detail EventMapper, EventType, EventContext
* tracking_php_api.md: caution about buy event

---------

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* server description updated

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>
Co-authored-by: julitafalcondusza <julitafalcondusza@users.noreply.github.com>
Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* Added translation-related Twig Component groups (#3091)

* Doc for Quable (#3085)

* Renaming - part one

* Current status

* Install doc ready?

* Fixed build

* Configuration doc

* Removed Quable other

* Added product guide skeleton

* Guide

* Current status

* Customize product embeds

* Added missing search doc

* Attribute rendering

* Provided a list of attributes

* Fixed mkdocs build

* Self review

* Added product code limitatin

* Fix typos

* Removed asset mentions

* Self review

* Selfreview done

* Apply suggestion from @mnocon

* Fixed table rendering

* Added Quable API

* Removed Quable client package

* Adrien's review

* After review

* Apply suggestions from code review

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* Review changes

* Update docs/product_catalog/quable/install_quable.md

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* Update docs/product_catalog/add_remote_pim_support.md

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* Update docs/product_catalog/customize_product_embed_templates.md

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* Manual changes

* Apply suggestions from code review

Co-authored-by: Paweł Niedzielski <pawel.tadeusz.niedzielski@gmail.com>

* Manual review feedback

* Added doc for language configuration (#3128)

* Added doc for language configuration

* Apply suggestion from @mnocon

---------

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>
Co-authored-by: Paweł Niedzielski <pawel.tadeusz.niedzielski@gmail.com>

* Added doc for TaxonomyNoEntries & TaxonomySubtree (#3082)

* Added doc for TaxonomyNoEntries

* Added doc for Taxonomy subtree

* Fixed CS

* Apply suggestions from code review

Co-authored-by: julitafalcondusza <117284672+julitafalcondusza@users.noreply.github.com>
Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* Apply suggestion from @mnocon

* Rebuild

---------

Co-authored-by: julitafalcondusza <117284672+julitafalcondusza@users.noreply.github.com>
Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* IBX-11485: Update doc for Symfony 7.4 (#3098)

* Update doc for Symfny 7.4

* Added bundle entry

* Apply suggestion from @mnocon

* Added var/share mention

* Review feedback

* Apply suggestions from code review

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

---------

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* Product tour (#3065)

* Product tour doc skeleton

* Review feedback

* Vale

* Review feedback - part 2

* Added doc for the new config

* Review feedback

* Adjusted includes

* Wording

* Specified button name

* Removed TODO

* Help Center and Product tour enabled by default

* Added images and interactive demo

* Apply suggestions from code review

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>
Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Manual changes

* [TMP] Fix build

* Fixed build

* Reworded

---------

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>
Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Documented try_catch (and sql: execute) in migrations (#3060)

* Added doc for try_catch and sql in migrations

* Vale suggestions

* Vale suggestion

* Added missing code block

* Review feedback

* Apply suggestions from code review

Co-authored-by: julitafalcondusza <117284672+julitafalcondusza@users.noreply.github.com>

* Improved example and clarified migration and migratoin step usage

* Removed RN enties

---------

Co-authored-by: julitafalcondusza <117284672+julitafalcondusza@users.noreply.github.com>
Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* IBX-10998: Document Gemini connector (#3025)

* IBX-10998: Document Gemini connector

* Update docs/ai_actions/extend_ai_actions.md

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Implement reviewer comments

* Apply suggestions from code review

Co-authored-by: Marek Nocoń <mnocon@users.noreply.github.com>

* Add RN entry

* IBX-11401: Describe Gemini embeddings provider (#3120)

* IBX-11401: Describe Gemini embeddings provider

* Remove the RN entry

* Fied broken link

* Fixed event page name

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>
Co-authored-by: Marek Nocoń <mnocon@users.noreply.github.com>

* Added doc for additional parameter for ibexa_render (#3043)

* Added doc for additional parameter for ibexa_render

* Added update sections

* Update docs/update_and_migration/from_5.0/update_from_5.0.md

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Reworked RN drafts

* Update docs/release_notes/ibexa_dxp_v4.6.md

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Removed RN entries

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Product tour: Deptract+Rector fixes (#3136)

* deptrac.baseline.yaml: Ignore NotificationScenarioSubscriber
* NotificationScenarioSubscriber: Apply Rector suggestions
* customize_product_tour.md: Update hl_lines

* updated_at_criterion.md: Rm EOF blank lines

* Release 5.0.7 follow up (#3137)

* Added homepage mention

* Applied update doc suggestion

* Reworded Gemini connector

* Fixed link?

* search_api.md: Minor fixes

* Raptor Add/Update buy event (#3141)

* tracking_php_api.md: Rm caution w/ EventType::BUY
* recommendations_twig_functions: Add buy event, sort events
* tracking_php_api.md: More PHP API Ref links in mapping intro

---------

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>

* 5.0.7 update (#3143)

* Added DB update scripts

* Mention that ibexa:setup is deprecated

* Reordered

* Reworded

* Apply suggestions from code review

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Release 5.0.7 fixes (#3140)

* Highlight and wording fixes

* Fixes

* Apply suggestions from code review

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>
Co-authored-by: Marek Nocoń <mnocon@users.noreply.github.com>

* Extracted to separate yaml file

* Apply suggestion from @adriendupuis

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Release notes for v5.0.7 and v4.6.29 (#3133)

* Init next release notes w/ postponed

* mkdocs.yml: Increment latest_tag_*

* Symfony 7.4

#3098

* Apply suggestion from @adriendupuis

* Revert "Apply suggestion from @adriendupuis"

This reverts commit 519103f.

* ibexa_dxp_v5.0.md: Raptor connector LTS Update

* ibexa_dxp_v5.0.md: Raptor connector LTS Update (Format)

* Apply suggestion from vale

* Apply suggestions from @julitafalcondusza

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* ibexa_dxp_v5.0.md: translation components, AI in PB

* ibexa_dxp_v5.0.md: Narrowed link for Raptor connector

* ibexa_dxp_v5.0.md: Sort editions and add Quable

* ibexa_dxp_vX.Y.md: Taxonomy search criteria

* ibexa_dxp_v4.6.md: Taxonomy search criteria: fix links

* Comment include 'snippets/release_XY.md'

* Use absolute links in snippets/release_XY.md

* Add Quable to ibexa_dxp_v4.6.md

* Add Integrated help's Product tour

* Raptor connector isn't an LTS Update

* Raptor connector isn't an LTS Update

* Quable is an add-on

* ibexa_dxp_v5.0.md: REST API request body examples

* ibexa_dxp_v5.0.md: PHP API

* ibexa_dxp_v5.0.md: PHP API

* Quable PIM isn't for Commerce

* Move Quable PIM up, rm badges

* ibexa_dxp_v5.0.md: Fix link

* Quable isn't for 4.6

* Add PHP API draft to release notes

* Apply suggestion from @dabrt

* remove blur, set versions and date

* uncomment include_file

* Apply suggestions from code review

Co-authored-by: julitafalcondusza <117284672+julitafalcondusza@users.noreply.github.com>

* resync ibexa_dxp_v5.0.md and ibexa_dxp_v4.6.md

* Apply suggestions from code review

Co-authored-by: Marek Nocoń <mnocon@users.noreply.github.com>

* resync ibexa_dxp_v5.0.md and ibexa_dxp_v4.6.md

* Move AI Actions in blocks upper

* TaxonomySubtree isn't new to these release

* Update the next release date

---------

Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>
Co-authored-by: julitafalcondusza <117284672+julitafalcondusza@users.noreply.github.com>
Co-authored-by: Marek Nocoń <mnocon@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Made Date and time/Symbol attribute links version aware

* Update docs/release_notes/ibexa_dxp_v4.6.md

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Update docs/release_notes/ibexa_dxp_v4.6.md

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

* Fixed broken link

* Added PHP API links

* Added security note

---------

Co-authored-by: julitafalcondusza <117284672+julitafalcondusza@users.noreply.github.com>
Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>
Co-authored-by: julitafalcondusza <julitafalcondusza@users.noreply.github.com>
Co-authored-by: Tomasz Dąbrowski <64841871+dabrt@users.noreply.github.com>
Co-authored-by: Paweł Niedzielski <pawel.tadeusz.niedzielski@gmail.com>
adriendupuis added a commit that referenced this pull request May 4, 2026
Moved inside 5.0 in #101
Probably there because modified in #199 for 4.6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Doc needed The changes require some documentation QA approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants