Fix user lookup in authmodule

[kulmann]

In case the userName and userId are not matching, then the authToken consists of both as a concatenated string. The user lookup in the AuthModule used the full userId value from the authToken, while it should only check the userId-part from the DB value.

Example auth token in DB, where userId and userName are not matching (= "LDAP use case")

+------+------------------------------------------------------------------+-----------+-----------------------------------------------+------------+
| id   | token                                                            | client_id | user_id                                       | expires    |
+------+------------------------------------------------------------------+-----------+-----------------------------------------------+------------+
| 2363 | ************************* |        29 | bkulmann:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx | 1633523318 |
+------+------------------------------------------------------------------+-----------+-----------------------------------------------+------------+

origin of the bug

The concatenated version of the userId access token field was introduced in https://github.com/owncloud/oauth2/pull/286 - apparently the part in the AuthModule was missed in that PR. This PR fixes it.

cc & thanks for debugging this @dschmidt @C0rby

[CLAassistant]

All committers have signed the CLA.

[kulmann]

cc @vjt maybe relevant for your environment as well to apply this PR as a patch?

[sonarcloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

[jnweiger]

@kulmann how do you trigger the situation where the user_id is a colon conctenated thing?

In my ldap+oauth setup I get

MariaDB [owncloud]> select * from oc_oauth2_access_tokens;
+----+------------------------------------------------------------------+-----------+--------------------------------------+------------+
| id | token                                                            | client_id | user_id                              | expires    |
+----+------------------------------------------------------------------+-----------+--------------------------------------+------------+
|  1 | Cl29zx0j***************** |         1 | 99xxxxxx-xxxx-xxxx-xxxx-f5edf5d332f5 | 1634127463 |
|  2 | Bc2xZS2****************** |         1 | c2xxxxxx-xxxx-xxxx-xxxx-f5edf5d332f5 | 1634128171 |
|  3 | KygJ1am****************** |         1 | acxxxxxx-xxxx-xxxx-xxxx-55a4da3d3a76 | 1634128861 |
+----+------------------------------------------------------------------+-----------+--------------------------------------+------------+
3 rows in set (0.001 sec)

MariaDB [owncloud]> select * from oc_accounts;
+----+----------------------+--------------------------------------+--------------------------------------+--------------+-------+------------+--------------------------+-------------------------------------------------------------+-------+
| id | email                | user_id                              | lower_user_id                        | display_name | quota | last_login | backend                  | home                                                        | state |
+----+----------------------+--------------------------------------+--------------------------------------+--------------+-------+------------+--------------------------+-------------------------------------------------------------+-------+
|  1 | admin@oc.example.com | admin                                | admin                                | admin        | NULL  | 1634122834 | OC\User\Database         | /var/www/owncloud/data/admin                                |     1 |
|  2 | testy@example.com    | acxxxxxx-xxxx-xxxx-xxxx-55a4da3d3a76 | acxxxxxx-xxxx-xxxx-xxxx-55a4da3d3a76 | testy        | 1 TB  | 1634125261 | OCA\User_LDAP\User_Proxy | /var/www/owncloud/data/acxxxxxx-xxxx-xxxx-xxxx-55a4da3d3a76 |     1 |
|  3 | user1@example.com    | 99xxxxxx-xxxx-xxxx-xxxx-f5edf5d332f5 | 99xxxxxx-xxxx-xxxx-xxxx-f5edf5d332f5 | User One     | 66 MB | 1634124326 | OCA\User_LDAP\User_Proxy | /var/www/owncloud/data/99xxxxxx-xxxx-xxxx-xxxx-f5edf5d332f5 |     1 |
|  4 | user2@example.com    | c2xxxxxx-xxxx-xxxx-xxxx-f5edf5d332f5 | c2xxxxxx-xxxx-xxxx-xxxx-f5edf5d332f5 | User Two     | 66 MB | 1634124572 | OCA\User_LDAP\User_Proxy | /var/www/owncloud/data/c2xxxxxx-xxxx-xxxx-xxxx-f5edf5d332f5 |     1 |
|  5 | NULL                 | dbxxxxxx-xxxx-xxxx-xxxx-19652cf0a9d2 | dbxxxxxx-xxxx-xxxx-xxxx-19652cf0a9d2 | ftp data     | 66 MB |          0 | OCA\User_LDAP\User_Proxy | /var/www/owncloud/data/dbxxxxxx-xxxx-xxxx-xxxx-19652cf0a9d2 |     1 |
+----+----------------------+--------------------------------------+--------------------------------------+--------------+-------+------------+--------------------------+-------------------------------------------------------------+-------+
5 rows in set (0.001 sec)

[dschmidt]

https://github.com/owncloud/oauth2/pull/286/files#diff-70e5309fcd3311d771b3db9c93490ea270fd5894769093765ec37edb68e5dd9bR222

$user->getUserName() and $user->getUID() need to be different

I'm not sure but from the top of my head you might as well check the oc_sessions (or something ...) table...

[jnweiger]

Succeeded with the different getUserName() and getUID() via openldap config that has

objectClass: inetOrgPerson
objectClass: organizationalPerson
objectClass: ownCloud
objectClass: person
objectClass: posixAccount
objectClass: top
uid: marie
givenName: Marie
sn: Curie
cn: marie-curie
sAMAccountName: MarieCurie
displayName: Marie Skłodowska Curie
mail: marie@example.org
uidNumber: 20001
gidNumber: 30000

sAMAccountName propagates into getUserName, while getUID seems to get an internal UUID.

This is written to the database as

         16803 Query    INSERT INTO `oc_oauth2_auth_codes`(`code`,`client_id`,`user_id`,`expires`,`code_challenge`,`code_challenge_method`) VALUES('laP0Me8jmEiDNUsMViiRkz8spnA8osVhmSCY8E7nIv4HCCsI8Mqkv3L8LMjIiFDI',1,'MarieCurie:36a0d182-c470-103b-9d16-9bfeddd58623',1634729201,'TFKo_f11JU3Ep6Ds_B8HJ5Lxzzq4UzGmjwHVpea1OZQ','S256')
211020 13:17:50  16792 Query    select * from oc_oauth2_auth_codes
         16842 Query    INSERT INTO `oc_oauth2_auth_codes`(`code`,`client_id`,`user_id`,`expires`,`code_challenge`,`code_challenge_method`) VALUES('QgkMbTvVfBX8QS8Mdezrt6vNQxXM2W9C5ZuLBshJphMH2XNJ5frjy8TVUcfFw1fc',1,'MarieCurie:36a0d182-c470-103b-9d16-9bfeddd58623',1634729320,'eayORfAQ-lct4Brl-WzWOVXo-lGIEEIwmFYj96J6XO0','S256')
         16843 Query    SELECT * FROM `oc_oauth2_auth_codes` WHERE `code` = 'QgkMbTvVfBX8QS8Mdezrt6vNQxXM2W9C5ZuLBshJphMH2XNJ5frjy8TVUcfFw1fc'
         16843 Query    DELETE FROM `oc_oauth2_auth_codes` WHERE `id` = 40
211020 13:19:12  16792 Query    select * from oc_oauth2_auth_codes
         16952 Query    INSERT INTO `oc_oauth2_auth_codes`(`code`,`client_id`,`user_id`,`expires`,`code_challenge`,`code_challenge_method`) VALUES('jNoAI0yEWJsh998EmwZ8afji5bqXUBEW1wpmOKUUKTTs4P5pqNCuy3iJ16WhU1Z9',1,'AlbertEinstein:36a07214-c470-103b-9d15-9bfeddd58623',1634729637,'VDIsMGB9Dn4p3bo3huyiUQ6C33pVQufVehaewV0aCcQ','S256')
         16954 Query    SELECT * FROM `oc_oauth2_auth_codes` WHERE `code` = 'jNoAI0yEWJsh998EmwZ8afji5bqXUBEW1wpmOKUUKTTs4P5pqNCuy3iJ16WhU1Z9'
         16954 Query    DELETE FROM `oc_oauth2_auth_codes` WHERE `id` = 41

In case of user einstein, the entry is removed from the db immediately after being used. Strange. In case of user marie, multiple entries are written, only one is removed.

In any case: it works fine.