Skip to content

N°9954 - Login-basic incompatible with OAuth2 server - #1018

Open
eespie wants to merge 7 commits into
developfrom
issue/9954_-_Login-basic_incompatible_with_OAuth2_server
Open

N°9954 - Login-basic incompatible with OAuth2 server#1018
eespie wants to merge 7 commits into
developfrom
issue/9954_-_Login-basic_incompatible_with_OAuth2_server

Conversation

@eespie

@eespie eespie commented Aug 20, 2026

Copy link
Copy Markdown
Member

N°9954 - Login-basic incompatible with OAuth2 server

Copilot AI balanced review requested due to automatic review settings August 20, 2026 13:49
@CombodoApplicationsAccount CombodoApplicationsAccount added the internal Work made by Combodo label Aug 20, 2026
@eespie
eespie requested a review from odain-cbd August 20, 2026 13:49
@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes Basic-login mode detection so OAuth2 Bearer tokens are not automatically claimed by the Basic authentication plugin, and adds unit coverage for colon-bearing and opaque Bearer values.

  • Centralizes authorization-header parsing in GetAuthUserAndPassword().
  • Selects Basic mode only when a username/password-shaped value can be decoded.
  • Adds login-mode detection tests for two Bearer-header forms.

Confidence Score: 4/5

The PR should not merge until standard HTTP Basic authorization headers remain parseable.

The parser now strips seven characters from every authorization header, corrupting valid Basic credential payloads received through the authorization-header server variables.

Files Needing Attention: application/loginbasic.class.inc.php; tests/php-unit-tests/unitary-tests/application/LoginFSM/LoginBasicTest.php

Important Files Changed

Filename Overview
application/loginbasic.class.inc.php Refactors Basic-mode detection to parse credentials first, but the seven-character prefix removal corrupts standard Basic authorization headers.
tests/php-unit-tests/unitary-tests/application/LoginFSM/LoginBasicTest.php Adds mode-detection coverage for Bearer values, but does not cover the standard Authorization: Basic path affected by the parser change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Authorization header] --> B[GetAuthUserAndPassword]
    B --> C[Remove authorization prefix]
    C --> D[Base64 decode]
    D --> E{Decoded value contains colon?}
    E -->|Yes| F[Set login_mode to basic]
    E -->|No| G[Leave login mode unset]
    F --> H[Read and validate credentials]
Loading

Reviews (1): Last reviewed commit: "N°9954 - Login-basic incompatible with O..." | Re-trigger Greptile

Comment thread application/loginbasic.class.inc.php Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates basic-auth detection to avoid conflicts with OAuth2 Bearer tokens and adds coverage.

Changes:

  • Refactors credential extraction and mode detection.
  • Adds tests for authorization-header handling.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
application/loginbasic.class.inc.php Refines authentication header parsing.
tests/php-unit-tests/unitary-tests/application/LoginFSM/LoginBasicTest.php Adds login-mode detection tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +119 to +125
// Decode without Bearer
$sDecodedAuthorisation = base64_decode(substr($sAuthorization, 7));
if (str_contains($sDecodedAuthorisation, ':')) {
return explode(':', $sDecodedAuthorisation);
}

return null;
Comment thread tests/php-unit-tests/unitary-tests/application/LoginFSM/LoginBasicTest.php Outdated
} elseif (isset($_SERVER['REDIRECT_HTTP_AUTHORIZATION']) && !empty($_SERVER['REDIRECT_HTTP_AUTHORIZATION'])) {
Session::Set('login_mode', 'basic');
} elseif (isset($_SERVER['PHP_AUTH_USER'])) {
if (!is_null($this->GetAuthUserAndPassword())) {
Comment thread tests/php-unit-tests/unitary-tests/application/LoginFSM/LoginBasicTest.php Outdated
eespie and others added 4 commits August 21, 2026 09:59
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread tests/php-unit-tests/unitary-tests/application/LoginFSM/LoginBasicTest.php Outdated
@eespie
eespie requested a review from odain-cbd August 31, 2026 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Work made by Combodo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants