Steps to reproduce:

  1. Log out
  2. GET /oauth2/authorize?[request params]
  3. Do not log in
  4. GET /oauth2/authorize
  5. Login
  6. There is an error saying 'client_id' is missing

This happens in the wild. Consider scenario when I have a login form with option to login using social networks. I try to login using social network, but fail.

Then I enter correct credentials, but see the error.

Attached patch solves the issue

CommentFileSizeAuthor
#3 oauth2_server-2664640-3.patch620 bytespjcdawkins
#2 2664640.patch580 bytesvalthebald

Comments

valthebald created an issue. See original summary.

valthebald’s picture

valthebald
Location Sofia
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2664640.patch580 bytes
pjcdawkins’s picture

pjcdawkins commented
StatusFileSize
new oauth2_server-2664640-3.patch620 bytes

Yes, this is a regression in version 7.x-1.5 caused by #2491575: Race condition when enabling a module with endpoint and OAuth2 authentication

This patch is a bit more liberal about what parameters trigger overwriting the session: client_id or response_type.

dashohoxha’s picture

dashohoxha commented

I have a module (oauth2_loginprovider) that depends on oauth2_server. Every time that oauth2_server makes a new release, it will break it, and I have to patch oauth2_server to fix it. This is a big problem because Drupal always pushes you to upgrade, then you do a `drush up` and your production site is broken, and then people complain to me that my module is not working anymore.

Can you guys (@pjcdawkins) please stop making new releases without getting a green light from the community? What's the point of a new release that breaks production sites? In particular the last realese was just pointless at all (no new features, no nothing). If some people need to use the latest and greatest, they can use the dev branch. In any case, they can always pick up the patches that they need. But a release that breaks things is totally unacceptable. Please get matured and behave responsibly.

Thanks for your understanding.

  • pjcdawkins committed b5eb189 on 7.x-1.x 
    Issue #2664640 by valthebald, pjcdawkins: Unable to continue...
pjcdawkins’s picture

pjcdawkins commented
Status: Needs review » Fixed

@dashohoxha I see your frustration and I agree that module releases should be stable - and in this case:

  • there should be more time between the commit and the release (here it was 5 days)
  • ideally, there should be a test that catches this

I don't agree that the last release (1.5) was pointless - it had fixes for 2 serious and 1 potentially serious bug:

and it added a couple of minor features (https://www.drupal.org/node/2663924).

Of course I don't think 'drush up' (and accepting every upgrade it offers) is a good way to keep a production site up-to-date.

I've committed the patch above and I'll release it soon... given that it's a very small hotfix. If I or anyone else has time to write a regression test, that of course would be worth it.

dashohoxha’s picture

dashohoxha commented

You are right, `drush up` totally distroys my site while updating the core, I have to do the update manually.
It used to work well though. Recently I upgraded to drush 8, and it rewards me with this "feature".

How do I get notified when a new release is made? Is there something like "Watch" of GitHub?

pjcdawkins’s picture

pjcdawkins commented

I tend to use drush up -n - I use that to look at potential new versions, and then I go and read the release notes etc.

You can subscribe to a project's issues by email, but I don't see any option to get notified on new releases.

The 7.x-1.6 release is just being built: https://www.drupal.org/node/2675880

Status: Fixed » Closed (fixed)

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