Some identity providers provide tokens using POST parameters rather than url encoded GET parameters.

This is the recommended mode if using Azure's OpenID Connect.

The attached patch changes OpenIDConnectRedirectController.php to support both POST and GET.

CommentFileSizeAuthor
#7 openid_connect-support_form_post-2984506-1-07.patch3.52 KBslasher13
#2 openid_connect-support_form_post-2984506-1-D8.patch3.44 KBdrasgardian
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drasgardian created an issue. See original summary.

drasgardian’s picture

drasgardian CreditAttribution: drasgardian at Eighty Options commented
FileSize
openid_connect-support_form_post-2984506-1-D8.patch3.44 KB

patch attached.

Mario Steinitz’s picture

Mario Steinitz
Location Hong Kong Central
CreditAttribution: Mario Steinitz at SHORELESS Limited commented
Status: Active » Needs review

Thank you for your patch. I had a quick review and don't see a reason, why incoming POST responses should not be supported.

As your patch does not include any breaking changes,
+1 for RTBC.

eiriksm’s picture

eiriksm
he/him
Primary language Norwegian Bokmål
Location Norway
CreditAttribution: eiriksm at Ny Media AS, Violinist, Foreningen Drupal Norge commented
Status: Needs review » Reviewed & tested by the community

There is one coding standard error in the patch: There should be no blank line after an inline comment.

     // ensure that the user, not a malicious script, is making the request.
-    $query = $this->requestStack->getCurrentRequest()->query;
-    $state_token = $query->get('state');
+
+    $request = $this->requestStack->getCurrentRequest();
+    $state_token = $request->get('state');

Other than that, looks great!

finne’s picture

finne
Primary language Dutch
Location Amsterdam
CreditAttribution: finne at Dx Experts commented

I can confirm this works fine in our project.

b2f’s picture

b2f CreditAttribution: b2f at Axess Open Web Services commented

Also confirmed to work here with hybrid authentication flow and user infos stored in the id_token (from https://www.drupal.org/project/openid_connect/issues/2965594)

slasher13’s picture

slasher13
Primary language German
CreditAttribution: slasher13 as a volunteer and at Wolters Kluwer commented
FileSize
openid_connect-support_form_post-2984506-1-07.patch3.52 KB

re-roll

jcnventura’s picture

jcnventura CreditAttribution: jcnventura at 1xINTERNET commented
Status: Reviewed & tested by the community » Needs review

Without an interdiff, this loses RTBC.

slasher13’s picture

slasher13
Primary language German
CreditAttribution: slasher13 as a volunteer and at Wolters Kluwer commented
Status: Needs review » Reviewed & tested by the community

It's a clean re-roll without changes.

  • slasher13 authored bd4d9f2 on 8.x-1.x 
    Issue #2984506 by drasgardian, slasher13, eiriksm, Mario Steinitz, finne...
jcnventura’s picture

jcnventura CreditAttribution: jcnventura at 1xINTERNET commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.