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

remarks after testing the creation of users and groups via GraphApi #3247

Closed
2 of 5 tasks
ScharfViktor opened this issue Mar 1, 2022 · 8 comments · Fixed by #3251
Closed
2 of 5 tasks

remarks after testing the creation of users and groups via GraphApi #3247

ScharfViktor opened this issue Mar 1, 2022 · 8 comments · Fixed by #3251
Labels

Comments

@ScharfViktor
Copy link
Contributor

ScharfViktor commented Mar 1, 2022

how setup: #3167
method descriptions: https://github.com/owncloud/ocis/pull/3149/files

run ocis with env:

set env:

export GRAPH_IDENTITY_BACKEND=ldap \
export GRAPH_LDAP_URI=ldap://localhost \
export GRAPH_LDAP_BIND_DN="uid=admin,ou=users,dc=owncloud,dc=com" \
export GRAPH_LDAP_BIND_PASSWORD=admin \
export GRAPH_LDAP_USER_EMAIL_ATTRIBUTE=mail \
export GRAPH_LDAP_USER_NAME_ATTRIBUTE=uid \
export GRAPH_LDAP_USER_BASE_DN="ou=users,dc=owncloud,dc=com" \
export GRAPH_LDAP_GROUP_BASE_DN="ou=groups,dc=owncloud,dc=com" \
export GRAPH_LDAP_SERVER_WRITE_ENABLED="true" \
export IDP_LDAP_FILTER="(&(objectclass=inetOrgPerson)(objectClass=owncloud))" \
export IDP_LDAP_URI=ldap://localhost \
export IDP_LDAP_BIND_DN="uid=admin,ou=users,dc=owncloud,dc=com" \
export IDP_LDAP_BIND_PASSWORD="admin" \
export IDP_LDAP_BASE_DN="dc=owncloud,dc=com" \
export IDP_LDAP_LOGIN_ATTRIBUTE=uid \
export IDP_LDAP_UUID_ATTRIBUTE="ownclouduuid" \
export IDP_LDAP_UUID_ATTRIBUTE_TYPE=binary \
export PROXY_ACCOUNT_BACKEND_TYPE=cs3 \
export OCS_ACCOUNT_BACKEND_TYPE=cs3 \
export STORAGE_LDAP_HOSTNAME=localhost \
export STORAGE_LDAP_PORT=636 \
export STORAGE_LDAP_INSECURE="true" \
export STORAGE_LDAP_BASE_DN="dc=owncloud,dc=com" \
export STORAGE_LDAP_BIND_DN="uid=admin,ou=users,dc=owncloud,dc=com" \
export STORAGE_LDAP_BIND_PASSWORD=admin \
export STORAGE_LDAP_LOGINFILTER='(&(objectclass=inetOrgPerson)(objectclass=owncloud)(|(uid={{login}})(mail={{login}})))' \
export STORAGE_LDAP_USERFILTER='(&(objectclass=inetOrgPerson)(objectclass=owncloud)(|(ownclouduuid={{.OpaqueId}})(uid={{.OpaqueId}})))' \
export STORAGE_LDAP_USERATTRIBUTEFILTER='(&(objectclass=owncloud)({{attr}}={{value}}))' \
export STORAGE_LDAP_FINDFILTER='(&(objectclass=owncloud)(|(uid={{query}}*)(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)(description={{query}}*)))' \
export STORAGE_LDAP_GROUPFILTER='(&(objectclass=groupOfUniqueNames)(objectclass=owncloud)(ownclouduuid={{.OpaqueId}}*))' \
export OCIS_INSECURE=true \
export OCIS_RUN_EXTENSIONS=settings,storage-metadata,graph,graph-explorer,ocs,store,thumbnails,web,webdav,storage-frontend,storage-gateway,storage-userprovider,storage-groupprovider,storage-authbasic,storage-authbearer,storage-authmachine,storage-users,storage-shares,storage-public-link,storage-appprovider,storage-sharing,proxy,idp,nats 

run ocis localy:
OCIS_LOG_PRETTY=true OCIS_LOG_COLOR=true ocis/bin/ocis server

Test result:

Users:

  • GET /me - OK

  • GET /users/{id} - OK

  • GET /users/{accountname} - OK

  • Create users POST /users:

    • create new user - OK
    • create user with same displayName - 200 is not possible win OC10 with same name
    • create user with same email - 200 - it is possible also in oc10 but why?
    • create user with invalid mail address - 200. expect 400 -Invalid mail address
    • create user with same onPremisesSamAccountName - 500 Entry Already Exists. Expect somenthing like 400 or 409 name already exist
    • create user without displayname email or onPremisesSamAccountName is not possible - OK we get 400 "Missing Required Attribute"
    • if you create user without password or with space in password, you get 200, but user cannot login GET https://localhost:9200/ocs/v1.php/cloud/capabilities?format=json 401 the reason is the same email. see Remarks
    • user (not admin) tries to create new user - 401
  • Update atribute single user PATCH users/{id}:

    • change display name - 200
    • change mail to valid/invalid value - 200
    • change mail to existing - 200
    • change onPremisesSamAccountName - 500 changing the user name is currently not supported
    • change password - 200 works
  • Delete user DELETE /users/{id} - OK

Groups:

  • Create groups POST /groups:

    • create new group - 200 OK
    • create group with existing name - 500 Entry Already Exists.
    • create with empty name - OK 400 Missing Required Attribute
    • user (not admin) cannot create group - 401
  • Change atribute single group PATCH groups/{id}:

    • change name - does not work
  • Add single member to group POST /groups/{id}/members/$ref:

    • existing user - 204
    • not existing - 404
    • admin adds himself to the group - OK
  • Add multiple users PATCH /groups/{id}:

    • existing users - OK
    • add single user - OK
    • add one user to different groups - OK
    • admin adds himself to the group - OK
  • GET /groups - OK

  • GET /groups/{groupid} - OK

  • GET /groups/{groupid}/members - OK

  • Delete groups members DELETE /groups/{groupid}/members/{id}/$ref - OK

Remarks:

  • There is no check that the email is correct. Expect - 400 Invalid mail address

  • Creating or changing a user with an existing onPremisesSamAccountName. Actual - 500 Entry Already Exists. Expect - 400 account with $name exist

  • Creating group with an existing name. Actual - 500 Entry Already Exists. Expect - 400 group with $name exist

  • Changing group name does not work

  • Creating a user with the same email will break login:

  1. create a new user1. try to log in to ocis - successfully
  2. create a new user 2 with same email. try to log in to ocis - unsuccessful. see screen

image

3. try to log in to ocis as user1 - 401 unsuccessfully. you create new user with admin email "[email protected]" - will not be able to continue working

Questions:

@ScharfViktor
Copy link
Contributor Author

@rhafer great work and thanks for the good documentation.
I finished the manual testing. please see my comments and questions

@rhafer
Copy link
Contributor

rhafer commented Mar 2, 2022

Test result:

Users:

...

  • create user with some displayName - 200 is not possible win OC10 with some name

Can you clarify what you mean by this? Can you paste the request body that you used here. And do you thing there is a bug?

  • create user with some email - 200 - it is possible also in oc10 but why?

What do you mean with some email?

  • create user with invalid mail address - 200. expect 400 -Invalid mail address
  • create user with some onPremisesSamAccountName - 500 Entry Already Exists. Expect somenthing like 400 or 409 name already exist

Yeah, those are bugs that I need to fix.

  * if you create user without password or with space in password, you get 200, but user cannot login GET https://localhost:9200/ocs/v1.php/cloud/capabilities?format=json 401

Haven't been able to fully reproduce this yet. Need to take a closer look.

...

Groups:

...

Remarks:

* There is no check that the email is correct. Expect - 400 Invalid mail address

* Creating or changing a user with an existing onPremisesSamAccountName. Actual - 500 Entry Already Exists. Expect - 400 account with $name exist

* Creating group with an existing name. Actual - 500 Entry Already Exists. Expect - 400 group with $name exist

* Changing group name does not work

Questions:

* creating users with the same email is correct?

You mean creating to users having the same email address? That's a good question. But actually I don't think we can't fully prevent that (at least when using the LDAP backend)

* how we get a list of users with groups? like
  ![Screenshot 2022-03-01 at 16 30 27](https://user-images.githubusercontent.com/84779829/156198076-c5e06fa5-6c83-4b01-8d16-2a3f57c102c0.png)

Currently that is not implemented. We need to add support to the graph users API for this kind of query.

* on some requests https://localhost:9200/graph-explorer/ looks like an error

That's a know issue in the graph-explorer. It doesn't handle the 204 Status code properly

@ScharfViktor
Copy link
Contributor Author

Can you clarify what you mean by this? Can you paste the request body that you used here. And do you thing there is a bug?
I understood that it was not a mistake. here is all right. In oC10 we cannot create users with same userName like onPremisesSamAccountName.

What do you mean with some email?
I apologize for my English. I meant "same email".

Haven't been able to fully reproduce this yet. Need to take a closer look.
I have added steps to the remarks on how to reproduce this. Problem is same email. When I asked you to tell me how to drop the DB, I created a new user with an email like the admins email

@ScharfViktor
Copy link
Contributor Author

Another important point, to know about it

we cannot share resources with users, because request https://localhost:9200/ocs/v2.php/apps/files_sharing/api/v1/sharees?search=sam&itemType=folder&page=1&perPage=200&format=json does not found users or group.

Steps:

  • admin creates user1
  • admin creates group1 and adds user1 to group
  • admin uploads file and tries to share file to user1 or group1

Actual: user or group not found
Screenshot 2022-03-02 at 15 02 01

rhafer added a commit to rhafer/ocis that referenced this issue Mar 2, 2022
This copies the validation code from the accounts service, also fixing a
bug in the regex that allowed adding mail addresses with whitespace and
other problematic characters to the domain part of the mail address.

Partial fix for: owncloud#3247
rhafer added a commit to rhafer/ocis that referenced this issue Mar 2, 2022
This copies the validation code from the accounts service, also fixing a
bug in the regex that allowed adding mail addresses with whitespace and
other problematic characters to the domain part of the mail address.

Partial fix for: owncloud#3247
@ScharfViktor
Copy link
Contributor Author

some bugs are fixed, but some still exist

@ScharfViktor ScharfViktor reopened this Mar 2, 2022
@rhafer
Copy link
Contributor

rhafer commented Mar 3, 2022

Another important point, to know about it

we cannot share resources with users, because request https://localhost:9200/ocs/v2.php/apps/files_sharing/api/v1/sharees?search=sam&itemType=folder&page=1&perPage=200&format=json does not found users or group.

I guess this is a configuration issue. Can you try adding

export STORAGE_LDAP_USERFINDFILTER='(&(objectclass=owncloud)(|(uid={{query}}*)(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)(description={{query}}*)))'

to your startup configuration. With that it should at least be able to list users. Note: the group part might still not work, that's because of know issues in the CS3 groupprovider.

@ScharfViktor
Copy link
Contributor Author

I tested it. Sharing with users works correctly. Thanks

labkode pushed a commit to cernbox/ocis that referenced this issue Mar 3, 2022
This copies the validation code from the accounts service, also fixing a
bug in the regex that allowed adding mail addresses with whitespace and
other problematic characters to the domain part of the mail address.

Partial fix for: owncloud#3247
rhafer added a commit to rhafer/ocis that referenced this issue Mar 3, 2022
This copies the validation code from the accounts service, also fixing a
bug in the regex that allowed adding mail addresses with whitespace and
other problematic characters to the domain part of the mail address.

Partial fix for: owncloud#3247
@ScharfViktor
Copy link
Contributor Author

Basically all remarks are corrected, I close in favor of #3516

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants