Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[full-ci] idp/proxy: Match users by ID instead of name by default #6338

Merged
merged 1 commit into from
May 23, 2023

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented May 17, 2023

Reconfigure the oidc clients for lico, so that lico adds the "lg.uuid" to tokens and userinfo by default. That claim will contain the userid. So we can now use the userid for matching users when using the default idm/idp configuration. This fixes further problems so that users being recreated with the same name are correctly treated as differnt users.

Fixes: #904

@rhafer
Copy link
Contributor Author

rhafer commented May 17, 2023

If this fix correct it should fail in CI with an unexpected success: https://github.com/owncloud/ocis/blob/master/tests/acceptance/expected-failures-webUI-on-OCIS-storage.md?plain=1#LL77C1-L79C1

@rhafer
Copy link
Contributor Author

rhafer commented May 17, 2023

@micbar I am a bit unsure if we should still take this for 3.0 (mainly because of the changed defaults in the proxy, which might cause some fallout in existing PROXY_AUTOPROVISION_ACCOUNTS=true setups.

@rhafer
Copy link
Contributor Author

rhafer commented May 17, 2023

Hm, this breaks basic auth it seems 😭

@rhafer
Copy link
Contributor Author

rhafer commented May 17, 2023

Hm, this breaks basic auth it seems sob

Another configuration issue (the cfg.UserCS3Claim and cfg.UserOIDCClaim were not passed to the basic auth middleware) hould be fixed now.

@rhafer
Copy link
Contributor Author

rhafer commented May 17, 2023

If this fix correct it should fail in CI with an unexpected success: https://github.com/owncloud/ocis/blob/master/tests/acceptance/expected-failures-webUI-on-OCIS-storage.md?plain=1#LL77C1-L79C1

Still does not work. Need to figure out what exactly that test does...

@rhafer
Copy link
Contributor Author

rhafer commented May 17, 2023

@SwikritiT The test how fails in the same way as features/webUILogin/openidLogin.feature:50 so I guess this is related to owncloud/web#4677 ?

  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 (2389ms).
  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 305 milliseconds.
√ Element <input[autocomplete="kopano-account username"]> was not present after 139 milliseconds.
√ Element <#files-view> was visible after 609 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 24 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
√ Element <//h2[text()="Logged out"]> was visible after 153 milliseconds.
    When the user exits the login error page
√ Element <#exitAnchor> was visible after 28 milliseconds.
    Then the user should be redirected to the login page
Timed out while waiting for element <//button//span[text()='Login' or text()='Log in']> to be present for 10000 milliseconds. - expected "visible" but got: "not found" (10010ms)
undefined    ✖ failed
      Timed out while waiting for element <//button//span[text()='Login' or text()='Log in']> to be present for 10000 milliseconds. - expected "visible" but got: "not found" (10010ms)
          at Proxy.waitForPage (/drone/src/webTestRunner/tests/acceptance/pageObjects/loginPage.js:20:21)
          at World.<anonymous> (/drone/src/webTestRunner/tests/acceptance/stepDefinitions/loginContext.js:103:34)

At least the user is now correctly redirected to the login error page. It just does not get back to the login page. Same as for the openidLogin.feature:50 test. So IMO #904 should be considered fixed with this.

@SwikritiT
Copy link
Contributor

SwikritiT commented May 18, 2023

@rhafer I tested this PR locally and it works as expected. As you mentioned the failure of the test should be related to owncloud/web#4677, I'll update the link in expected to fail. Thanks!

Reconfigure the oidc clients for lico, so that lico adds the "lg.uuid" to
tokens and userinfo by default. That claim will contain the userid. So
we can now use the userid for matching users when using the default
idm/idp configuration. This fixes further problems so that users being
recreated with the same name are correctly treated as differnt users.

Fixes: owncloud#904
@sonarcloud
Copy link

sonarcloud bot commented May 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@rhafer rhafer merged commit 52951b4 into owncloud:master May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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