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

no enable/disable user provisioning API endpoint #1420

Closed
individual-it opened this issue Jul 31, 2020 · 18 comments
Closed

no enable/disable user provisioning API endpoint #1420

individual-it opened this issue Jul 31, 2020 · 18 comments
Assignees

Comments

@individual-it
Copy link
Member

just to track it in the tests

@haribhandari07 haribhandari07 transferred this issue from owncloud/ocis-ocs Jan 25, 2021
@phil-davis phil-davis changed the title no enable/disable endpoint no enable/disable user provisioning API endpoint Feb 22, 2021
@settings settings bot removed the p3-medium label Apr 7, 2021
@stale
Copy link

stale bot commented Jun 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status:Stale label Jun 6, 2021
@stale stale bot closed this as completed Jun 16, 2021
@stale stale bot removed the Status:Stale label Jun 18, 2021
@stale
Copy link

stale bot commented Aug 17, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

@wkloucek
Copy link
Contributor

@individual-it @phil-davis Could you please have a look at

How can they pass, if we don't have the enable / disable user provisioning API endpoint?

Context: we had a PR with some changes which triggered these two tests to fail and therefore I looked at them.

@wkloucek wkloucek reopened this Nov 19, 2021
@phil-davis
Copy link
Contributor

phil-davis commented Nov 19, 2021

Note:
apiProvisioning-v1/disableUser.feature:256 and
apiProvisioning-v2/disableUser.feature:253
are already in expected-failures. Those test with the "old" dav_version of the public WebDAV API

apiProvisioning-v1/disableUser.feature:257 and
apiProvisioning-v2/disableUser.feature:254
are the examples for "new" dav_version of the public WebDAV API

I will look at why they were passing previously...

@phil-davis phil-davis self-assigned this Nov 19, 2021
@phil-davis
Copy link
Contributor

The test happens to be in pipeline Core-API-Tests-ocis-storage-2
Example: https://drone.owncloud.com/owncloud/ocis/7950/23/6

  Scenario Outline: getting public link share shared by disabled user using the new public WebDAV API                                                                                                            # /srv/app/testrunner/tests/acceptance/features/apiProvisioning-v1/disableUser.feature:247
    Given user "Alice" has been created with default attributes and small skeleton files                                                                                                                         # FeatureContext::userHasBeenCreatedWithDefaultAttributes()
    And user "Alice" has created a public link share with settings                                                                                                                                               # FeatureContext::userHasCreatedAPublicLinkShareWithSettings()
      | path        | /textfile0.txt |
      | permissions | read           |
    When the administrator disables user "Alice" using the provisioning API                                                                                                                                      # FeatureContext::adminDisablesUserUsingTheProvisioningApi()
    Then the public should be able to download the last publicly shared file using the <dav_version> public WebDAV API without a password and the content should be "ownCloud test text file 0" plus end-of-line # PublicWebDavContext::checkLastPublicSharedFileDownloadPlusEndOfLine()

    Examples:
      | dav_version |
      | old         |
        The downloaded content was expected to be 'ownCloud test text file 0
        ', but actually is ''. HTTP status was 401
        Failed asserting that two strings are equal.
        --- Expected
        +++ Actual
        @@ @@
        -'ownCloud test text file 0\n
        -'
        +''
      | new         |

@wkloucek
Copy link
Contributor

But how can When the administrator disables user "Alice" using the provisioning API succeed, if this API doesn't exist?

@phil-davis
Copy link
Contributor

phil-davis commented Nov 19, 2021

But how can When the administrator disables user "Alice" using the provisioning API succeed, if this API doesn't exist?

When steps are only coded to fail if some exception happens with the test code (the network to the system-under-test goes down or...)

A test is supposed to check the evidence about whether the When step operated correctly by using Then steps. Often we have When steps that attempt to do something (download a file), and actually the When step code cannot/does not know if the API call is expected to succeed or not. The When step code saves the response (status, body etc) and Then steps should assert things about it, and things about other observable behavior of the system.

In this case the test is a "negative" test. On oC10, when a user is disabled, any public shares made by the user still work. (Whether that should be the case or not is a product requirement decision - but that is what happens now). So the test is confirming that if the user is disabled, then "nothing happens" to the public shares.

The test scenario should really have more Then steps that check some basic stuff about the When step, like:
Then the HTTP status should be xxx
Then the user should be disabled

That will make it fail earlier, when it discovers that the "disable" API call failed.

The difference in failure at the moment is some artifact of which old and new public WebDAV API are working.

I will adjust core tests (and have a look at other core tests that do not have enough Then checks)

@phil-davis
Copy link
Contributor

See owncloud/core#39512 for details of refactoring of tests - that is a separate issue to this one.

@stale
Copy link

stale bot commented Jan 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status:Stale label Jan 21, 2022
@wkloucek
Copy link
Contributor

@phil-davis can we close this, since owncloud/core#39612 is merged?

@stale stale bot removed the Status:Stale label Jan 21, 2022
@stale
Copy link

stale bot commented Mar 23, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status:Stale label Mar 23, 2022
@phil-davis
Copy link
Contributor

@phil-davis can we close this, since owncloud/core#39612 is merged?

The enable/disable user tests still fails. I guess that the feature has not yet been implemented?

@SwikritiT
Copy link
Contributor

Now we do user-related operations through graph api is this issue relevant now?

cc @phil-davis @individual-it

@individual-it
Copy link
Member Author

I would suggest to make sure no tests in ocis rely on enable/disable users by the provisioning API and delete all tests that try to test those things (if any)

@micbar do you agree?

@micbar
Copy link
Contributor

micbar commented Jul 4, 2023

We can enable/disable users via graph now.

IMO there is just a config missing in the default product.

@rhafer
Copy link
Contributor

rhafer commented Jul 5, 2023

IMO there is just a config missing in the default product.

It should work with the default configuration (just checked that).

@individual-it @SwikritiT Which exact test is this failing?

@SwikritiT
Copy link
Contributor

SwikritiT commented Jul 6, 2023

@individual-it @SwikritiT Which exact test is this failing?

No tests are failing as per my knowledge, we also don't have this issue linked to the expected-to-fail file. But still, sometimes tests get linked to some other issues. This issue specifically is for ocs provisioning API but as we are using graph API for this operation we can close this issue. But like @individual-it mentioned we need to check that there aren't any tests that rely on provisioning API to do this and remove them if there are.

@rhafer
Copy link
Contributor

rhafer commented Oct 5, 2023

Enable/Disable users is implemented in the graph API. And AFAIK all tests are now using that.

@rhafer rhafer closed this as completed Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants