[QA] Browser session is valid for newly created user of same name

jnweiger commented 3 years ago

Setup via docker-compose-eos-test.yml from branch fix-yml-for-rc5 on localhost From the WEB UI:

moss creates user testy
in a different browser session, testy logs in, and creates some folders or files.
moss deletes user testy
the browser session of testy no longer works, error messages appear.
moss creates a new testy user
the browser session of the original testy works again, the new contents is seen.

This is a corner case, but may lead to access to a wrong account's data.

Expected behaviour: sessions remain invalidated forever, once the user is deleted.

jnweiger commented 3 years ago

@C0rby maybe exploitable like this:

assume an intrudor got control of a user account
admin deletes and recreates the user account to kick out the intrudor
intrudor changes the accounts password before the real user can access. ... don't know what els can be done with that :-)

C0rby commented 3 years ago

Oooh, that is interesting and tricky since oidc is a stateless mechanism AFAIK and there is no central "session" we could invalidate. This requires some research.

exalate-issue-sync[bot] commented 3 years ago

David Christofas commented: So after a bit of research I found the following options:

We could include the accounts uuid in the jwts (only possible if the access token is a jwt) and on every request check that the affected user's id matches.
The idp needs to hold a blacklist with invalidated access tokens. This requires the idp to synchronize the blacklist if it is setup in a distributed manner. Also when an admin deletes a user the admin has no access to the users access token and therefore can not tell the idp which token to invalidate.

Both options are not really satisfying. I think this issue requires some more brainstorming together with the team.

exalate-issue-sync[bot] commented 3 years ago

Jörn Friedrich Dreyer commented: optimal solution: idp sends the ownclouduuid property that we can rely on. if the idp cannot send arbitrary claims (eg konnectd): when creating accounts we need to store the sub+iss with the account (our accounts have an identities property that can be used for that). then we can double check if the sub+iss still matches on logins.

exalate-issue-sync[bot] commented 3 years ago

Ilja Neumann commented: Contacted upstream with the proposal of [~jfd]. Meanwhile we could quickly implement this feature in a fork and use that until the change has landed it upstream. We done this before.

individual-it commented 3 years ago

[x] ToDo QA-team: think about if we can test this kind of scenario and if yes add tests

SwikritiT commented 1 year ago

This issue still exists

create a user test
as the user test login
delete the user test
re-create the user test
reload the browser of the previously logged-in user test
the page is in loading state but reload again and you're logged in as a newly created user

https://user-images.githubusercontent.com/41103328/207509556-e2909368-cdd2-4375-8ab1-4dfb7c779544.mp4

started ocis with docker image: owncloud/ocis:latest

rhafer commented 1 year ago

This is a configuration issue of lico. By default its LDAP backend uses a hash of the LDAP DN of the user entry as the sub claim of a user. This means that the user will get the same sub claim after being recreated.

It is possible however to configure a custom LDAP attribute for use as the sub claim in lico (via the LDAP_SUB_ATTRIBUTES env var for lico). We should just set that to the same value as the UUID attribute (defaulting to owncloudUUID ). That way the user would get a different sub claim after being recreated. (As it would get a new UUID value).

Fix is in the works ...

SwikritiT commented 1 year ago

There's a UI test in expected to fail linked with this issue https://github.com/owncloud/web/blob/43c9c45b0e66add3077cd360f9e7680db67bf8db/tests/acceptance/expected-failures-with-ocis-server-ocis-storage.md?plain=1#LL70C1-L70C119

TODO QA TEAM

[ ] check if the issue has been fixed
[ ] enable the test again

SwikritiT commented 1 year ago

@rhafer I can still reproduce the error.

https://github.com/owncloud/ocis/assets/41103328/3129b972-4410-4455-a6a0-dce08fd967ed

Here's the log

2023/05/17 10:49:46 http: TLS handshake error from [::1]:32806: remote error: tls: unknown certificate
2023/05/17 10:49:46 http: TLS handshake error from [::1]:32814: remote error: tls: unknown certificate
2023/05/17 10:49:46 http: TLS handshake error from [::1]:32824: remote error: tls: unknown certificate
2023/05/17 10:51:42 http: proxy error: context canceled
{"level":"error","service":"storage-users","pkg":"rgrpc","traceid":"00000000000000000000000000000000","error":"xattr.get /home/swikriti/.ocis/storage/users/spaces/7b/fca149-9e76-4b58-a0f0-4489c9b0bf6b/nodes/7b/fc/a1/49/-9e76-4b58-a0f0-4489c9b0bf6b user.ocis.name: no data available","id":"7bfca149-9e76-4b58-a0f0-4489c9b0bf6b","time":"2023-05-17T10:52:02.436918991+05:45","message":"could not read node"}
{"level":"error","service":"storage-users","pkg":"rgrpc","traceid":"00000000000000000000000000000000","error":"xattr.get /home/swikriti/.ocis/storage/users/spaces/7b/fca149-9e76-4b58-a0f0-4489c9b0bf6b/nodes/7b/fc/a1/49/-9e76-4b58-a0f0-4489c9b0bf6b user.ocis.name: no data available","status":{"code":15,"message":"error listing spaces","trace":"00000000000000000000000000000000"},"filters":[{"type":2,"Term":{"Id":{"opaque_id":"544a71c4-7950-4512-a388-0b244f4c9554$7bfca149-9e76-4b58-a0f0-4489c9b0bf6b!7bfca149-9e76-4b58-a0f0-4489c9b0bf6b"}}},{"type":4,"Term":{"SpaceType":"+grant"}}],"time":"2023-05-17T10:52:02.436937216+05:45","message":"failed to list storage spaces"}
2023/05/17 10:53:09 http: proxy error: context canceled
{"level":"error","service":"storage-users","pkg":"rgrpc","traceid":"00000000000000000000000000000000","error":"xattr.get /home/swikriti/.ocis/storage/users/spaces/d0/db3d6b-f78a-4eaf-b993-e5417237691c/nodes/d0/db/3d/6b/-f78a-4eaf-b993-e5417237691c user.ocis.name: no data available","id":"d0db3d6b-f78a-4eaf-b993-e5417237691c","time":"2023-05-17T10:53:22.43680465+05:45","message":"could not read node"}
{"level":"error","service":"storage-users","pkg":"rgrpc","traceid":"00000000000000000000000000000000","error":"xattr.get /home/swikriti/.ocis/storage/users/spaces/d0/db3d6b-f78a-4eaf-b993-e5417237691c/nodes/d0/db/3d/6b/-f78a-4eaf-b993-e5417237691c user.ocis.name: no data available","status":{"code":15,"message":"error listing spaces","trace":"00000000000000000000000000000000"},"filters":[{"type":2,"Term":{"Id":{"opaque_id":"544a71c4-7950-4512-a388-0b244f4c9554$d0db3d6b-f78a-4eaf-b993-e5417237691c!d0db3d6b-f78a-4eaf-b993-e5417237691c"}}},{"type":4,"Term":{"SpaceType":"+grant"}}],"time":"2023-05-17T10:53:22.436845491+05:45","message":"failed to list storage spaces"}

Vesraion: 3.0.0-rc.3+158971b97
Web clint version: 7.0.0-rc.37

I built the ocis from commit id: 158971b97c4ae6e34c1109a0c76bbd7a8d9bded2

Can you try if it is reproducible in your system?

SwikritiT commented 1 year ago

The related test is also failing in CI https://drone.owncloud.com/owncloud/ocis/22484/70/13, if this is the expected behaviour

 @openIdLogin @ocis-konnectd-issue-26
  Scenario: the user session of a deleted user should not be valid for newly created user of same name # features/webUILogin/openidLogin.feature:60
- Connecting to selenium on port 4444...

ℹ Connected to selenium on port 4444 (2478ms).
  Using: chrome (104.0.5112.79) on Linux platform.

    Given user "Alice" has been created with default attributes and without skeleton files in the server
    Given user "Alice" has logged in using the webUI
√ Element <input[autocomplete="kopano-account username"]> was visible after 304 milliseconds.
√ Element <input[autocomplete="kopano-account username"]> was not present after 167 milliseconds.
√ Element <#files-view> was visible after 656 milliseconds.
√ Element <//*[self::table[contains(@class, "files-table")] or self::div[contains(@class, "files-empty")] or self::div[contains(@class, "files-not-found")]]> was visible after 31 milliseconds.
    And user "Alice" has been deleted in the server
    And user "Alice" has been created with default attributes and without skeleton files in the server
    When the user reloads the current page of the webUI
    Then the user should be redirected to the login error page
Timed out while waiting for element <//h2[text()="Logged out"]> to be present for 10000 milliseconds. - expected "visible" but got: "not found" (10014ms)
undefined    ✖ failed
      Timed out while waiting for element <//h2[text()="Logged out"]> to be present for 10000 milliseconds. - expected "visible" but got: "not found" (10014ms)
          at Proxy.waitTillLoaded (/drone/src/webTestRunner/tests/acceptance/pageObjects/loginErrorPage.js:20:40)
          at World.<anonymous> (/drone/src/webTestRunner/tests/acceptance/stepDefinitions/loginContext.js:115:39)
    When the user exits the login error page
    - skipped
    Then the user should be redirected to the login page
    - skipped

rhafer commented 1 year ago

Hm, this is weird, it worked for me. I'll take another look.

rhafer commented 1 year ago

Hm, I guess I understand what's going on. My fix only addressed a part of the problem. Before my fix the recreated user was still able to get its accesstoken refreshed. With the fix merged that refresh will be no longer possible (as the "sub" of the recreated user change).

However, if the user is recreated while the accesstoken is still valid the session stays active. (The user won't have access to anything however).

We can fix this and immediately kill the session by further reconfiguring the IDP to add the owncloud UUID to the claims and move away from matching user by username to match them by the ID in the proxy.

C8opmBM commented 1 year ago

Since my last update of ocis last week I believe, I am no longer able to login (access denied). Im using Authelia, and my setup has been working for a long time. I see no breaking changes, but now I'm being asked for a claim, which I never used. I've been using this guide since the beginning of the year without problems: https://helgeklein.com/blog/owncloud-infinite-scale-with-openid-connect-authentication-for-home-networks/

Now I'm being spammed with:

`ERR claim not set or empty claim=lg.uuid claims={"amr":["pwd","otp","mfa"],"aud":["xdXOt13JKxym1B1QcEncf2XDkLAexMBFwiT9j6EfhhHFJhs2KM9jbjTmf8JBXE69","xdXOt13JKxym1B1QcEncf2XDkLAexMBFwiT9j6EfhhHFJhs2KM9jbjTmf8JBXE69"],"auth_time":1680888917,"azp":"xdXOt13JKxym1B1QcEncf2XDkLAexMBFwiT9j6EfhhHFJhs2KM9jbjTmf8JBXE69","client_id":"xdXOt13JKxym1B1QcEncf2XDkLAexMBFwiT9j6EfhhHFJhs2KM9jbjTmf8JBXE69","email":"web@irvi.net","email_verified":true,"iat":1683262123,"iss":"https://auth.xxx.net","name":"status quo","preferred_username":"xxxx","rat":1683262123,"sub":"95527cef-dac1-4407-a3be-4b613e8fc541"} line=github.com/owncloud/ocis/v2/services/proxy/pkg/middleware/account_resolver.go:63 service=proxy

I searched the docs, I've tried to add various options such the ones below, to no avail. Can you please advise?

      PROXY_USER_OIDC_CLAIM: "email"
      PROXY_ROLE_ASSIGNMENT_DRIVER: oidc
      PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM: roles
      PROXY_USER_OIDC_CLAIM: preferred_username

Using ocis web with docker, windows client and the android client. All latest versions.

S-Panta commented 1 year ago

Although a newly created user with the same name couldn't access the data of the previously created user of the same name, the browser session still remains logged in even after you change the entire credential (including password). Only when you change the username will the session redirects to logout. When every detail of new user are changed except username

https://github.com/owncloud/ocis/assets/64484313/a147062d-c64d-4ab1-b3ae-c50b51eff86a

When username is changed while keeping every detail same

https://github.com/owncloud/ocis/assets/64484313/998d496c-c685-4d38-8cfb-1494347f254a

rhafer commented 9 months ago

To really fix this, I think we need to address https://github.com/libregraph/lico/issues/98 first. After that we can bring back the original fix for the issue: https://github.com/owncloud/ocis/commit/52951b42b0db11f652e3924ba73cec3b68666042

dragonchaser commented 9 months ago

@micbar does it make sense to tackle this, since we are planning to move to another idp?

DeepDiver1975 commented 8 months ago

As just discussed and agreed on:

the access token has a life time of 5 minutes
the user cannot perform critical operations like change password
the user has no access to any space

-> the old user has at most 5 mins to view personal files of the new user and these files first need to be uploaded -> this is a very unlikely attack vector and even if the information leakage is pretty low

BUT: this scenario has to be retested when the idp is changed

owncloud / ocis

[QA] Browser session is valid for newly created user of same name #904