Redirect URI has the language prefix in it (in D10)

I've run into this issue and have tried to get a conversation going on 2883450 about this.

I tested the patch and it seems like the language prefix is still there at least by judging from the preview of the redirect URL.

I have the redirect URL displayed https://domain.com/fi/openid-connect/windows_aad.

This also breaks our current integration after updating the module because the redirect URL's change. I will check the issue 🐛 Missing url prefix on language neutral content Fixed ?

Hi,

The above patch (3383036-1-openid-connect-redirect-url-options-array-update.patch) isn't worked as expected they changed language none in response validation. I included it in request as well. It's working fine for me. Please validate and let me know if you face any issues.

Thinks,
Buvaneswaran.

In addition: the change should also applied to OpenIDConnectClientFormBase::getRedirectUrl() for the "correct" display in settings form.

I was fighting with the same problem as well. Rerolled against recent 3.x-dev + included feedback from #6

Within the Form /src/Form/OpenIDConnectClientFormBase.php, the logout url is also showing the language code within the path. Should not it be changed to?

Kobe Wright → made their first commit to this issue’s fork.

Adding a patch to use against 3.0.0-alpha2

I was too trigger happy with removing the language manager, other modules might depend on it (as I discovered with openid_connect_windows_aad).
Adding a new patch that keeps it.

Reroll of #13 for the dev branch.

Hi Team,

I have applied the patch #14, but still getting language code in redirect_uri= parameter.
I have also tried with patch #13, it fixed the language code issue for redirect_uri parameter. But once redirected, the URL appears like this http:///en-connect.

Anyone have solution for this issue?

@baaluaanand Check if you have created a custom client plugin extending OpenIDConnectClientBase. It was the same case for me as well. If thats the case, you need to update your custom plugin for the methods you are overloading.

I created a new patch refining the code in #14. Since languageManager is not being used anymore, I removed the references for it in my patch.

The patch from #16 is missing one more removal, that leads to PHP error. I am attaching updated version.

Hello, I guess this patch will be embarked in alpha3 version ?

@Salvor_Hardin Yes, if someone marks it "Reviewed and tested by community".

I've a similar problem in D10 but with de v1.4. I've updated v1.2 to v1.4 and no change secret_id secret_client for google and when I try to connect, I've this message "Access blocked: This app’s request is invalid, Error 400: redirect_uri_mismatch"

Tested on a couple of sites. Marking as RTBC.

As per my earlier comment #13 I want to reiterate that I explicitly added the language manager again after realising that other plugins (like the one from openid_connect_windows_aad, see https://git.drupalcode.org/project/openid_connect_windows_aad/-/blob/2.0...) still use it, I see that the latest patches are removing it again.

While I myself prefer to clean up unnecessary dependencies as well, we do have to consider the impact for other modules / users.
Not saying we shouldn't delete it but just wondering if it's worth removing it here/now knowing that this will require patching downstream.
What do you guys think?

@Kobe Wright, In my opinion - though this is related, this is still a different project/subproject: https://www.drupal.org/project/openid_connect_windows_aad →

I would create an issue ticket there → and link this issue as a related.

Version 3.x of the module is in alpha, so we can indeed screw around with the API as we see fit, and yes the AAD module is indeed a concern.

One question however is about the other option for having multi-language sites in Drupal.. What if instead of path prefix, a site is configured to have a different domain per language? Can it be that the new 'path_processing => FALSE' line is sufficient to prevent the path prefix from being added, but would still allow for the different domain?

@jcnventura I've just created a custom patch this morning about that for v1.4 and create now this issue https://www.drupal.org/project/openid_connect/issues/3417174 🐛 Redirect URI has the language prefix in it (v1.4 with D10) Needs review

Seems like the final solution should be what @Joouul posted in the other issue:

https://www.drupal.org/files/issues/2024-01-26/openid_connect-3417174-01... →

I closed the other issue as a duplicate. This needs to be marked RTBC using the simpler solution, and checking that Drupal will switch to alternate domains when using language-specific domains, while still not adding the path prefix when using that solution.

There is an update with 'language' option

This patch for 3.0.0 Alpha2

The issue still needs to target 3.x-dev. Not the version for which you created a patch.

This issue also occurs on OpenID Connect 2.0.0-beta7

BramDriesen → changed the visibility of the branch 3383036-redirect-uri-has to hidden.

Cleaned up all the patches/branches and created a fresh branch as this marked as the "final solution" in #26 and the discussion around that one.

Also not sure if we need to pass the 'path_processing' => FALSE to the URL Options in the following call: $authorization_endpoint = Url::fromUri($endpoints['authorization'], $url_options)->toString(TRUE); (line 286 in OpenIDConnectClientBase)

I can confirm that the MR from #30 works for me.

I tested on:

Drupal 10.2.3
openid_connect 3.0.0-alpha3

Hi here is a patch for openid_connect 3.0.0-alpha3, I am not sure what is happening with the MR request.

Thanks

I am not sure what is happening with the MR request.

Just use MR 99

I confirm #26 works.

It won't go anywhere as long as the status is "Needs review".

Hi,

Patch #36 worked for me with 3.0.0-alpha6. Drupal 10.4.5.

Thank you.

Moving this to RTBC. The code change is small and MR 99 still apply. Multiple people have confirmed it works for them though we have no details about what site config they had.

• #34 have not been answered, but the change seem to address the problem for those that have it.
• Do we need tests?