Hi,

I know the normal behaviour of Drupal Commerce is to display a 404 page is the user doesn't have access to the checkout page but we need to display a 403 in our case.

So here is the corresponding patch.

CommentFileSizeAuthor
#16 redirect_users_to_403-2605012-16.patch2.03 KBvisabhishek
#14 commerce_checkout.pages-403-instead-of-404-2605012-14.patch2.37 KBjamesquinton
#12 commerce_checkout.pages-403-instead-of-404-2605012-12.patch1.99 KBjamesquinton
#9 commerce_checkout.pages-403-instead-of-404-2605012-9.patch2.02 KBjamesquinton
#7 commerce_checkout.pages-403-instead-of-404-2605012-7.patch2.16 KBjamesquinton
#2 commerce_checkout.pages-403-instead-of-404-2605012-2.patch872 bytesMarvine

Comments

Marvine created an issue. See original summary.

Marvine’s picture

Marvine commented
StatusFileSize
new commerce_checkout.pages-403-instead-of-404-2605012-2.patch872 bytes
visabhishek’s picture

visabhishek commented
Status: Active » Needs review

Patch Looks good, so i am changing status to Needs Review

Status: Needs review » Needs work

The last submitted patch, 2: commerce_checkout.pages-403-instead-of-404-2605012-2.patch, failed testing.

rszrama’s picture

rszrama commented
Version: 7.x-1.11 » 7.x-1.x-dev
Category: Bug report » Feature request
Status: Needs work » Active

This is a breaking change that we wouldn't make in a point release. The only possible option here would be a setting to make the response configurable.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Active » Needs work 
+++ b/modules/checkout/includes/commerce_checkout.pages.inc
@@ -17,11 +17,9 @@ function commerce_checkout_router($order, $checkout_page = NULL) {
-    return MENU_NOT_FOUND;
+    return MENU_ACCESS_DENIED;

Keep comment in place. Try, not sure on variable name. Just dumped something.

variable_get('commerce_checkout_access_response_type', MENU_NOT_FOUND);

jamesquinton’s picture

jamesquinton commented
Status: Needs work » Needs review
StatusFileSize
new commerce_checkout.pages-403-instead-of-404-2605012-7.patch2.16 KB

Hi,

Just seen this issue and think that it makes good sense to return a 403. Based on Ryan's suggestion I've updated the checkout settings form with a new option, and return a response based on that.

Patch attached!

Status: Needs review » Needs work

The last submitted patch, 7: commerce_checkout.pages-403-instead-of-404-2605012-7.patch, failed testing.

jamesquinton’s picture

jamesquinton commented
StatusFileSize
new commerce_checkout.pages-403-instead-of-404-2605012-9.patch2.02 KB

2nd attempt...

jamesquinton’s picture

jamesquinton commented
Status: Needs work » Needs review
Marvine’s picture

Marvine commented

It looks good to me, great patch !

Couple of small comments :

We probably want to update the comment here to consider the new context :

 // could return a 403, but then the user would know they've identified a
   // potentially valid checkout URL.
   if (!commerce_checkout_access($order)) {
-    return MENU_NOT_FOUND;
+    return intval(variable_get('commerce_checkout_access_response_type', MENU_NOT_FOUND));
   }

Full stop needed :
// Save checkout access denied response

Full stop needed and a comma after '403' .

  // Select checkout access denied response
  $form['checkout_access_response_type'] = array(
    '#type' => 'select',
    '#title' => t('Checkout access denied response'),
    '#description' => t('Select the required response for users that attempt to
      view a checkout page for which they don\'t have access.'),
    '#default_value' => variable_get('commerce_checkout_access_response_type', MENU_NOT_FOUND),
    '#weight' => 1,
    '#options' => array(
      MENU_NOT_FOUND => '404',
      MENU_ACCESS_DENIED => '403'
jamesquinton’s picture

jamesquinton commented
StatusFileSize
new commerce_checkout.pages-403-instead-of-404-2605012-12.patch1.99 KB

Hi Marvine,

Good spot - I've updated the comments, and corrected the array syntax.

Status: Needs review » Needs work

The last submitted patch, 12: commerce_checkout.pages-403-instead-of-404-2605012-12.patch, failed testing.

jamesquinton’s picture

jamesquinton commented
Status: Needs work » Needs review
StatusFileSize
new commerce_checkout.pages-403-instead-of-404-2605012-14.patch2.37 KB

Status: Needs review » Needs work

The last submitted patch, 14: commerce_checkout.pages-403-instead-of-404-2605012-14.patch, failed testing.

visabhishek’s picture

visabhishek commented
Status: Needs work » Needs review
StatusFileSize
new redirect_users_to_403-2605012-16.patch2.03 KB

Hope this one apply

jamesquinton’s picture

jamesquinton commented

Yep, it should do. The paths are incorrect in my patch - sorry!