nextcloud / nextcloud/mail

OAuth redirect silently swallows provider errors: a failed authorization renders the same “Account connected” page as success, and logs nothing

Open
#13,269 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

1. to develop bug
Dominant language
JavaScript
Stars
1k
Forks
348
Avg merge
12h 28m
Merged PRs (30d)
91

Description

Steps to reproduce
  1. Configure Microsoft (Azure) or Google XOAUTH2 in the Mail admin settings.
  2. Start an account authorization from Mail, so the browser is sent to the provider.
  3. Cause the provider to redirect back with an error rather than a code — e.g. decline the consent screen, or let the app registration hit a Conditional Access policy. The provider then calls the redirect URI with ?error=access_denied&error_description=… and no code.
Expected behavior

The user is told the authorization failed, and the reason is recorded in nextcloud.log.

Actual behavior

The page renders "Account connected — You can close this window", identical to a successful authorization. Nothing is written to nextcloud.log. No token is stored.

The user has no way to distinguish a successful link from a failed one, and an administrator inspecting the logs afterwards finds no trace of the failure at all.

Cause

oauthRedirect() accepts an $error parameter and never reads it. Every outcome — success, missing code, unauthenticated session, and provider-returned error — renders the same oauth_done template:

https://github.com/nextcloud/mail/blob/a406c97ecacd25b220386c37c29adfe5bbed0d1a/lib/Controller/MicrosoftIntegrationController.php#L83-L102

public function oauthRedirect(?string $code, ?string $state, ?string $session_state, ?string $error): Response {
    if ($this->userId === null) {
        // TODO: redirect to main nextcloud page
        return new StandaloneTemplateResponse(Application::APP_ID, 'oauth_done', [], 'guest');
    }

    if (!isset($code, $state)) {
        // TODO: handle error
        return new StandaloneTemplateResponse(Application::APP_ID, 'oauth_done', [], 'guest');
    }
    try {
    ...

$error occurs exactly three times in the file: in the docblock, in the signature, and in the // TODO: handle error comment above the branch that ignores it. A provider error therefore lands in the !isset($code, $state) branch and is discarded.

Of the four exit paths, only two log anything (invalid OAuth state warns; finishConnect() failures log). The two silent ones are the two a user is most likely to hit.

GoogleIntegrationController::oauthRedirect() has the same signature and the same structure, so this affects both providers:

https://github.com/nextcloud/mail/blob/a406c97ecacd25b220386c37c29adfe5bbed0d1a/lib/Controller/GoogleIntegrationController.php#L79

Impact

This is mostly a diagnosability problem, but a sharp one. While debugging an unrelated reconnect failure on a live instance, this behaviour cost several rounds of investigation: the "Account connected" page was taken as evidence that authorization had succeeded, when in fact no token had been written. The only reliable signal was inspecting oauth_token_ttl and oauth_access_token in oc_mail_accounts directly.

Any user hitting a declined consent screen, an expired authorization code, or a tenant Conditional Access policy will be told their account is connected when it is not.

Suggested fix

In the !isset($code, $state) branch, when $error is present:

  • $this->logger->warning('OAuth authorization failed', ['error' => $error, 'error_description' => $errorDescription]) — note error_description would need adding to the signature; it carries the provider's AADSTS… code, which is the actually diagnostic part.
  • render an error state in the oauth_done template rather than the success text, so the user knows to retry.

Both controllers would need the change. Passing $error through to the template is enough; it does not need to be surfaced verbatim to the user.

Mail app version

5.10.7 (behaviour confirmed unchanged on main at a406c97ecacd25b220386c37c29adfe5bbed0d1a, 2026-07-08)

Nextcloud version

33.0.6

Mailserver or service

Microsoft 365 (outlook.office365.com, XOAUTH2). Google path affected by inspection, not tested.

Operating system

Debian (Nextcloud All-in-One container)

PHP engine version

PHP 8.3

Nextcloud memory caching

No response

Web server

Apache (supported)

Database

PostgreSQL

Additional info

Found while investigating a separate defect in the OAuth reconnect flow, filed separately: updateAccount() synchronises mailboxes before getUserConsent() runs, which makes reconnection impossible once a refresh token has expired. The two are independent — this one would still be worth fixing on its own, since it hides every provider-side authorization failure.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with oauthRedirect() in lib/Controller/MicrosoftIntegrationController.php and lib/Controller/GoogleIntegrationController.php, especially the branch handling a missing code or state. Trace how the oauth_done template displays success, then verify the provider error and description can be logged and represented as a failure. Done means both controllers distinguish provider failures from success and record the failure in nextcloud.log.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
authentication, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.