This one is as tricky as #3236329: Authorize session getting overridden to reproduce, but there are a couple of ways, of which the first one is the easiest:

Important: automatic authorization must be enabled to reproduce the 'access denied' screen.

  • Arrive at the login screen, go back immediately
  • Open a different tab, and login on the authorize server
  • Go back to other tab, hit open id connect: at this point you will be redirected and be presented with an access denied screen, because the state query parameter is the same as the first request, while the second attempt generated a new one.

It's kind of an edge case, but we also have this when a user needs to register first. In our case, email validation isn't required immediately, so you are authenticated as soon as you register. Users then go back to the original site, hit login again, but then end up with the access denied screen. We are trying to guide them with messages to redirect them to oauth/authorize, but you know, not everyone reads messages :)

The fix is relatively easy:


  if ($this->currentUser()->isAnonymous()) {
    // This part stays the same
  }
  else {
    // A user may be authenticated at this point (e.g. registration flow ..)
    if ($bridgeRequest->get('client_id')) {
      $_SESSION['oauth2_server_authorize'] = $bridgeRequest;
    }
  }

Issue fork oauth2_server-3471891

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

swentel created an issue. See original summary.

swentel’s picture

swentel commented
Issue summary: View changes
swentel’s picture

swentel commented
Issue summary: View changes

cafuego made their first commit to this issue’s fork.

swentel’s picture

swentel commented

Thinking about it some more the next day,

  • maybe 'automatic authorization' isn't necessarily needed for this to reproduce. Didn't test it yet, but if it's not enabled, the code in the else statement calling validateAuthorizeRequest() would fail and then immediately return a response, where it it should probably be able to actually connect.
  • My proposal for the fix could probably be easier as there's already the same 3 lines of code in the anonymous check. So this would probably be ok as well, covering all cases. 
    if ($bridgeRequest->get('client_id')) {
      $_SESSION['oauth2_server_authorize'] = $bridgeRequest;
    }

    if ($this->currentUser()->isAnonymous()) {
            $url = new Url('user.login', [], ['query' => ['destination' => Url::fromRoute('oauth2_server.authorize')->toString()]]);
      $url->setAbsolute(TRUE);
      return new RedirectResponse($url->toString());
    }

This means though that $_SESSION will always contain the 'oauth2_server_authorize' which might not be necessary at all. But that would also be the case in my original proposal, so maybe that one could be changed a bit like this:


  if ($this->currentUser()->isAnonymous()) {
    // This part stays the same
  }
  else {
    // A user may be authenticated at this point (e.g. registration flow ..)
    if ($bridgeRequest->get('client_id') && isset($_SESSION['oauth2_server_authorize'])) {
      $_SESSION['oauth2_server_authorize'] = $bridgeRequest;
    }
  }

Feedback welcome of course :)

swentel’s picture

swentel commented
Title: Authorize session getting overridden for authenticated users and automatic authorization » Stale authorize session used for authenticated users
Issue summary: View changes

Changing title as it makes more sense I think