Problem/Motivation

While using user logout API it gives a blank response with 204 status
http://site.com/user/logout?_format=json&token=<logout_token>

Steps to reproduce

1) Enable the required modules for web services (rest, hal..etc)
2) Login any user using rest (site.com/user/login?_format=json) and get the csrf_token and logout_token
3) hit the logout token (/user/logout?_format=json&token=<logout_token>)
4) It will give the blank output with 204 status code

Proposed resolution

core/modules/user/src/Controller/UserAuthenticationController.php -> logout() method we can just add the msg with logged out successfully.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#17 3169120-nr-bot.txt143 bytesneeds-review-queue-bot
#12 After_patch_apply.png21.23 KBvikashsoni
#12 Patch_apply.png70.57 KBvikashsoni
#12 Before_patch_apply.png18.18 KBvikashsoni
#8 interdiff-6_8.txt1.65 KBnaresh_bavaskar
#8 3169120-8.patch2.15 KBnaresh_bavaskar
#6 3169120-6.patch975 bytesayushmishra206
#3 3169120-3.patch970 bytesnaresh_bavaskar

Comments

naresh_bavaskar created an issue. See original summary.

naresh_bavaskar’s picture

naresh_bavaskar
Location Pune, Maharashtra
commented
Issue summary: View changes
naresh_bavaskar’s picture

naresh_bavaskar
Location Pune, Maharashtra
commented
Assigned: naresh_bavaskar » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3169120-3.patch970 bytes

Please review

Status: Needs review » Needs work

The last submitted patch, 3: 3169120-3.patch, failed testing. View results

anmolgoyal74’s picture

anmolgoyal74 commented

response HTTP code should be 204 for logout request.

ayushmishra206’s picture

ayushmishra206 commented
StatusFileSize
new 3169120-6.patch975 bytes
ayushmishra206’s picture

ayushmishra206 commented
Status: Needs work » Needs review
naresh_bavaskar’s picture

naresh_bavaskar
Location Pune, Maharashtra
commented
StatusFileSize
new 3169120-8.patch2.15 KB
new interdiff-6_8.txt1.65 KB

#6 patch was making logout response to 204 No content but the the purpose this issue was to have the content with any msg on logout response.
Please review the updated patch phpunit testcases updation.

samiullah’s picture

samiullah commented

For login route I m getting {"message":"Not acceptable format: json"}

Are there any detailed steps to test this one

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

vikashsoni’s picture

vikashsoni commented
StatusFileSize
new Before_patch_apply.png18.18 KB
new Patch_apply.png70.57 KB
new After_patch_apply.png21.23 KB

@naresh_bavaskar apply #8 patch working fine now getting response when logged out in rest api " Logged out successfully " sharing screenshot ....

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Issue tags: +ContributionWeekend2022

Thanks @vikashsoni for testing the patch. In the future, you don't need to include a screenshot of applying the patch. Also, you can embed your screenshots within your comment so it's easier for others to see the results.

I have read a bit about whether or not 200 or 204 should be returned and found this helpful: https://itqna.net/questions/3533/what-http-verb-i-use-logout

A successful response SHOULD be 200 (OK) if the response includes an entity describing the status, 202 (Accepted) if the action has been enacted, or 204 (No Content) does not include an entity.

So, if I'm understanding correctly, it seems the 204 was indeed correct when no message was included, but should be changed to 200 with the message added.

That said, I'm concerned that changing the function signature of logout won't be allowed without handling backwards-compatibility.

+++ b/core/modules/user/src/Controller/UserAuthenticationController.php
@@ -299,12 +299,17 @@ protected function userLoginFinalize(UserInterface $user) {
+  public function logout(Request $request) {

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3169120-nr-bot.txt143 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

d34dman’s picture

d34dman commented

> So, if I'm understanding correctly, it seems the 204 was indeed correct when no message was included, but should be changed to 200 with the message added.

@Kristen Pol, Thanks for clearing this up. The documentation over here does mention that it is a 204 https://www.drupal.org/docs/8/core/modules/rest/javascript-and-drupal-8-....

However, the change record over here https://www.drupal.org/node/2720655 could be miss-leading, as that is what is causing confusion.

EDIT:

miss-leading part being, it says the response for logout -- gives "Logged out!". It also doesn't mention that it would be a 204, instead of 200.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.