chore: bump version to 0.6.2

.github/workflows/dist.yml

@@ -0,0 +1,17 @@
+name: Build dist
+
+on:
+  push:
+    branches: [ release-0.6 ]
+  pull_request:
+    types:
+      - opened
+      - synchronize
+      - reopened
+
+permissions:
+  contents: read
+
+jobs:
+  build:
+    uses: owncloud/reusable-workflows/.github/workflows/build.yml@main

.github/workflows/lint-pr-title.yml

@@ -0,0 +1,16 @@
+name: Lint PR title
+
+on:
+  pull_request:
+    types:
+      - opened
+      - edited
+      - synchronize
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: amannn/action-semantic-pull-request@48f256284bd46cdaab1048c3721360e808335d50 # v6.1.1
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/main.yml

@@ -0,0 +1,71 @@
+name: CI
+
+on:
+  push:
+    branches:
+      - release-0.6
+  pull_request:
+    types:
+      - opened
+      - synchronize
+      - reopened
+
+permissions:
+  contents: read
+
+concurrency:
+  group: ci-${{ github.ref }}
+  cancel-in-progress: true
+
+env:
+  APP_NAME: oauth2
+  PHP_VERSIONS: '["7.4"]'
+  # The 0.6 line supports ownCloud 10 only (see appinfo/info.xml), so the tests
+  # must run against the 10.x core branch - owncloud/core master is 11.x by now.
+  CORE_REF: '10.16'
+
+jobs:
+  get-vars:
+    runs-on: ubuntu-latest
+    outputs:
+      app-name: ${{ env.APP_NAME }}
+      php-versions: ${{ env.PHP_VERSIONS }}
+      core-ref: ${{ env.CORE_REF }}
+    steps:
+      - name: Set variables
+        run: |
+          echo "App name $APP_NAME"
+          echo "PHP versions string: $PHP_VERSIONS"
+          echo "Core ref: $CORE_REF"
+
+  semantic-git-messages:
+    name: Commits
+    uses: owncloud/reusable-workflows/.github/workflows/semantic-git-message.yml@main
+
+  php-code-style:
+    name: PHP Code Style
+    needs:
+      - get-vars
+    uses: owncloud/reusable-workflows/.github/workflows/php-codestyle.yml@main
+    with:
+      app-name: ${{ needs.get-vars.outputs.app-name }}
+      php-versions: ${{ needs.get-vars.outputs.php-versions }}
+      core-ref: ${{ needs.get-vars.outputs.core-ref }}
+      core-ref-php74: ${{ needs.get-vars.outputs.core-ref }}
+
+  php-unit:
+    name: PHP Unit
+    needs:
+      - get-vars
+    uses: owncloud/reusable-workflows/.github/workflows/php-unit.yml@main
+    with:
+      app-name: ${{ needs.get-vars.outputs.app-name }}
+      php-versions: ${{ needs.get-vars.outputs.php-versions }}
+      core-ref: ${{ needs.get-vars.outputs.core-ref }}
+      core-ref-php74: ${{ needs.get-vars.outputs.core-ref }}
+
+  # No acceptance job on this branch: the reusable acceptance workflow installs a
+  # hardcoded owncloud-daily-master-qa.tar.bz2 (11.x) with the owncloudci/core:php83
+  # image, and an app declaring max-version="10" cannot be enabled on that server.
+  # Once acceptance.yml gains inputs for the server tarball and installer image, add
+  # the acceptance-webui job here as on master.

.github/workflows/release.yml

@@ -0,0 +1,22 @@
+name: Release
+
+on:
+  push:
+    tags:
+      - 'v*'
+
+permissions:
+  contents: read
+
+jobs:
+  release:
+    permissions:
+      contents: write
+    uses: owncloud/reusable-workflows/.github/workflows/release.yml@main
+    with:
+      app-name: oauth2
+      artifact-glob: build/dist/oauth2.tar.gz
+    secrets:
+      SIGNING_KEY: ${{ secrets.SIGNING_KEY }}
+      SIGNING_CERT: ${{ secrets.SIGNING_CERT }}
+      SIGNING_CHAIN: ${{ secrets.SIGNING_CHAIN }}

.github/workflows/security-scan.yml

@@ -0,0 +1,32 @@
+name: Security Scan
+
+on:
+  push:
+    branches:
+      - release-0.6
+  pull_request:
+    types:
+      - opened
+      - synchronize
+      - reopened
+  schedule:
+    # Weekly, so CVEs published against already-released code surface without a
+    # code change. GitHub only runs schedules on the default branch, so this is
+    # inert here - it is kept for symmetry with master.
+    - cron: '0 3 * * 1'
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+concurrency:
+  group: security-scan-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  trivy:
+    name: Trivy
+    uses: owncloud/reusable-workflows/.github/workflows/trivy.yml@main
+    with:
+      # The 0.6 line targets php 7.4, so build the tree to scan with 7.4.
+      php-version: '7.4'

CHANGELOG.md

@@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
+## [0.6.2] - 2026-09-09
+
+### Fixed
+
+- [#389](https://github.com/owncloud/oauth2/pull/389) - fix: treat the OAuth2 user id as opaque
+
+
 ## [0.6.1] - 2023-08-31
 
 ### FIXED
@@ -154,7 +161,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Verify Bearer token even if the session is still valid - [#53](https://github.com/owncloud/oauth2/pull/53)
 - Use displayname on switch user screen - [#90](https://github.com/owncloud/oauth2/pull/90)
 
-[Unreleased]: https://github.com/owncloud/oauth2/compare/v0.6.1...master
+[Unreleased]: https://github.com/owncloud/oauth2/compare/v0.6.2...release-0.6
+[0.6.2]: https://github.com/owncloud/oauth2/compare/v0.6.1...v0.6.2
 [0.6.1]: https://github.com/owncloud/oauth2/compare/v0.6.0...v0.6.1
 [0.6.0]: https://github.com/owncloud/oauth2/compare/v0.5.3...v0.6.0
 [0.5.3]: https://github.com/owncloud/oauth2/compare/v0.5.2...v0.5.3

appinfo/info.xml

@@ -24,7 +24,7 @@ When using OAuth2 a unique access token is generated for each device or third pa
 - [OAuth protocol web page](https://oauth.net/2/)</description>
     <licence>AGPL</licence>
     <author>Project Seminar "sciebo@Learnweb" of the University of Münster, Thomas Müller</author>
-    <version>0.6.1</version>
+    <version>0.6.2</version>
     <namespace>OAuth2</namespace>
     <category>security</category>
     <website>https://github.com/owncloud/oauth2</website>

lib/AuthModule.php

@@ -113,11 +113,7 @@ public function authToken($bearerToken): ?IUser {
 
 		/** @var \OCP\IUserManager $userManager */
 		$userManager = $container->query('UserManager');
-		$userId = $accessToken->getUserId();
-		if (\strstr($userId, ':')) {
-			list(1 => $userId) = \explode(':', $userId, 2);
-		}
-		return $userManager->get($userId);
+		return $userManager->get($accessToken->getUserId());
 	}
 
 	protected function tokenCanBeHandledByOpenIDConnect(): bool {

lib/Controller/OAuthApiController.php

@@ -192,11 +192,6 @@ public function generateToken(
 
 				$userId = $authorizationCode->getUserId();
 
-				// strip off username if it exists
-				if (\strstr($userId, ':')) {
-					list(, $userId) = \explode(':', $userId, 2);
-				}
-
 				$this->authorizationCodeMapper->delete($authorizationCode);
 
 				$userObj = $this->userManager->get($userId);
@@ -235,11 +230,6 @@ public function generateToken(
 
 				$userId = $refreshToken->getUserId();
 
-				// strip off username if it exists
-				if (\strstr($userId, ':')) {
-					list(, $userId) = \explode(':', $userId, 2);
-				}
-
 				$userObj = $this->userManager->get($userId);
 				if ($userObj === null || !$userObj->isEnabled()) {
 					$this->logger->debug("the matching user is missing or disabled", ['app'=>__CLASS__]);

tests/unit/AuthModuleTest.php

@@ -37,9 +37,6 @@ class AuthModuleTest extends TestCase {
 	/** @var String $userId */
 	private $userId = 'john';
 
-	/** @var String $userIdConcat */
-	private $userIdConcat = 'John Doe:john';
-
 	/** @var ClientMapper $clientMapper */
 	private $clientMapper;
 
@@ -118,18 +115,18 @@ public function testAuth() {
 		$user = $this->authModule->auth($request);
 		$this->assertNotNull($user);
 		$this->assertEquals($this->userId, $user->getUID());
+	}
 
-		// Valid request with ConcatUserID
-		$request = $this->getMockBuilder(IRequest::class)->getMock();
-		$this->accessToken->setUserId($this->userIdConcat);
-		$this->accessToken = $this->accessTokenMapper->update($this->accessToken);
-		$request->expects($this->once())
-			->method('getHeader')
-			->with($this->equalTo('Authorization'))
-			->will($this->returnValue('Bearer ' . $this->accessToken->getToken()));
-		$user = $this->authModule->auth($request);
-		$this->assertNotNull($user);
-		$this->assertEquals($this->userId, $user->getUID());
+	/**
+	 * The user id stored on the token is opaque - a colon in it must not be
+	 * treated as a "login name:user id" separator, otherwise the token
+	 * authenticates a different account than the one it was issued for.
+	 */
+	public function testAuthTokenKeepsUserIdWithColon() {
+		$this->accessToken->setUserId('attacker:' . $this->userId);
+		$this->accessTokenMapper->update($this->accessToken);
+
+		$this->assertNull($this->authModule->authToken($this->accessToken->getToken()));
 	}
 
 	/**

tests/unit/Controller/OAuthApiControllerTest.php

@@ -625,6 +625,76 @@ public function testGenerateTokenWithAuthorizationCodeAndPKCES256() {
 		$this->assertNotEmpty($json->token_type);
 		$this->assertEquals('Bearer', $json->token_type);
 	}
+	/**
+	 * The stored user id is opaque - a colon in it must not be treated as a
+	 * "login name:user id" separator, otherwise the token is issued for a
+	 * different account than the one which authorized the code.
+	 */
+	public function testGenerateTokenWithAuthorizationCodeKeepsUserIdWithColon() {
+		$_SERVER['PHP_AUTH_USER'] = $this->clientIdentifier1;
+		$_SERVER['PHP_AUTH_PW'] = $this->clientSecret;
+
+		$userIdWithColon = 'attacker:' . $this->userId;
+		$this->authorizationCode->setUserId($userIdWithColon);
+		$this->authorizationCodeMapper->update($this->authorizationCode);
+
+		$requestedUserIds = [];
+		$this->mockUserManagerFor($userIdWithColon, $requestedUserIds);
+
+		$result = $this->controller->generateToken(
+			'authorization_code',
+			$this->authorizationCode->getCode(),
+			$this->redirectUri
+		);
+		$this->assertEquals(200, $result->getStatus());
+		$json = \json_decode($result->render());
+		$this->assertEquals([$userIdWithColon], $requestedUserIds);
+		$this->assertEquals($userIdWithColon, $json->user_id);
+		$this->assertEquals($userIdWithColon, $this->accessTokenMapper->findByToken($json->access_token)->getUserId());
+		$this->assertEquals($userIdWithColon, $this->refreshTokenMapper->findByToken($json->refresh_token)->getUserId());
+	}
+
+	/**
+	 * @see testGenerateTokenWithAuthorizationCodeKeepsUserIdWithColon
+	 */
+	public function testGenerateTokenWithRefreshTokenKeepsUserIdWithColon() {
+		$_SERVER['PHP_AUTH_USER'] = $this->clientIdentifier1;
+		$_SERVER['PHP_AUTH_PW'] = $this->clientSecret;
+
+		$userIdWithColon = 'attacker:' . $this->userId;
+		$this->refreshToken->setUserId($userIdWithColon);
+		$this->refreshTokenMapper->update($this->refreshToken);
+
+		$requestedUserIds = [];
+		$this->mockUserManagerFor($userIdWithColon, $requestedUserIds);
+
+		$result = $this->controller->generateToken('refresh_token', null, null, $this->refreshToken->getToken());
+		$this->assertEquals(200, $result->getStatus());
+		$json = \json_decode($result->render());
+		$this->assertEquals([$userIdWithColon], $requestedUserIds);
+		$this->assertEquals($userIdWithColon, $json->user_id);
+		$this->assertEquals($userIdWithColon, $this->accessTokenMapper->findByToken($json->access_token)->getUserId());
+		$this->assertEquals($userIdWithColon, $this->refreshTokenMapper->findByToken($json->refresh_token)->getUserId());
+	}
+
+	/**
+	 * Lets the user manager resolve $userId only, and records every requested id.
+	 *
+	 * @param string $userId The only user id which resolves to an enabled user.
+	 * @param array $requestedUserIds Collects the requested user ids.
+	 * @return void
+	 */
+	private function mockUserManagerFor($userId, array &$requestedUserIds) {
+		$userMock = $this->createMock(IUser::class);
+		$userMock->method('isEnabled')->willReturn(true);
+
+		$this->userManager->method('get')
+			->willReturnCallback(function ($requestedUserId) use ($userId, $userMock, &$requestedUserIds) {
+				$requestedUserIds[] = $requestedUserId;
+				return $requestedUserId === $userId ? $userMock : null;
+			});
+	}
+
 	public function testGenerateTokenWithAuthorizationCodeAndPKCESInvalidChallengeMethod() {
 		$_SERVER['PHP_AUTH_USER'] = $this->clientIdentifier1;
 		$_SERVER['PHP_AUTH_PW'] = $this->clientSecret;