Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Need user id as UUID adjustments #41

Closed
PVince81 opened this issue Aug 12, 2020 · 7 comments · Fixed by #44
Closed

Need user id as UUID adjustments #41

PVince81 opened this issue Aug 12, 2020 · 7 comments · Fixed by #44
Labels
bug Something isn't working

Comments

@PVince81
Copy link
Contributor

BLOCKS owncloud/ocis#409

Steps

  1. Check out Update deps 2020 07 22 ocis#409
  2. Add redigo go.mod replace replace github.com/gomodule/redigo => github.com/gomodule/redigo v1.8.2
  3. Start ocis
  4. Check out Phoenix locally (master)
  5. Compile Phoenix with yarn build-all
  6. Kill phoenix ocis service, CLI in ocis repo `bin/ocis-debug kill phoenix)
  7. Start ocis-phoenix from its repo with PHOENIX_ASSET_PATH=~/work/workspace/phoenix/dist PHOENIX_WEB_CONFIG=~/work/workspace/phoenix/config-ocis.json bin/ocis-phoenix --log-level debug server and point at the correct settings
  8. Login in web UI as einstein/relativity
  9. Check network console for "value-list" and see that it's fine.
  10. Now in the Phoenix repo, setup test runner according to the docs but without LDAP
  11. yarn run acceptance-tests tests/acceptance/features/webUICreateFilesFolders/createFolderEdgeCases.feature:58
  12. In the middle of the test after log in, hit Ctrl+C
  13. Now in the web UI, log out and log in as "user1/1234"
  14. Check network console

Expected result

Call "value-list" has no error.
No notification.

Actual result

Notification "Failed to load settings"
400 Bad request for "value-list" call: account_uuid: must be a valid UUID.

OCIS log:

2020-08-12T12:38:11+02:00 DBG director found path=/api/v0/settings/values-list policy=reva prefix=/api/v0/settings routeType=prefix service=proxy
account_uuid:"user1"
2020-08-12T12:38:11+02:00 DBG  bytes=36 duration=0.187233 method=POST path=/api/v0/settings/values-list proto=HTTP/1.1 request=a9c7b18a-684d-492c-98fc-5feba690cef7 service=settings status=400

so in the case of "user1" it's using "user1" as the UUID instead of a true uuid.

Maybe the Phoenix tests setup is not creating the test users properly ?

@individual-it @kulmann

@PVince81 PVince81 added the bug Something isn't working label Aug 12, 2020
@kulmann
Copy link
Contributor

kulmann commented Aug 12, 2020

github.com/owncloud/ocis-pkg/v2 v2.2.2-0.20200527082518-5641fa4a4c8c
needs to be updated

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 12, 2020

ocis-accounts on-disk show that user1 was created by user id:

accounts
├── 4c510ada-c86b-4815-8820-42cdf82c3d51
├── 820ba2a1-3f54-4538-80a4-2d73007e30bf
├── 932b4540-8d16-481e-8ef4-588e4b6b151c
├── bc596f3c-c955-4328-80a0-60d018b4ad57
├── f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c
└── user1

with user1 entry:

{"id":"user1","accountEnabled":true,"preferredName":"user1","mail":"user1@example.org","passwordProfile":{"password":"$6$5VmhT3WyJrmW988H$pgaH6GkNTBVLIJpR.DulQe5c3Re1cvrq7M4BbMSO8umezjDwXc7CIa2OdP/x2x./cRPCpAf2DOyFZTqGPPftW."},"onPremisesSamAccountName":"user1"}

this is the einstein entry:

{"id":"4c510ada-c86b-4815-8820-42cdf82c3d51","accountEnabled":true,"displayName":"Albert Einstein","preferredName":"einstein","uidNumber":"20000","gidNumber":"30000","mail":"einstein@example.org","passwordProfile":{"password":"$6$rounds=35210$sa1u5Pmfo4cr23Vw$RJNGElaDB1D3xorWkfTEGm2Ko.o2QL3E0cimKx23MNxVWVFSkUUeRoC7FqC4RzYDNQBD6cKzovTEaDD.8TDkD."},"memberOf":[{"id":"509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"},{"id":"6040aa17-9c64-4fef-9bd0-77234d71bad0"},{"id":"dd58e5ec-842e-498b-8800-61f2ec6f911f"},{"id":"262982c1-2362-4afa-bfdf-8cbfef64a06e"}],"onPremisesSamAccountName":"einstein"}

@PVince81
Copy link
Contributor Author

PR for the update #42

@kulmann
Copy link
Contributor

kulmann commented Aug 12, 2020

If alphanumeric account uuids are expected / allowed, we need to adapt the input validation in ocis-settings. It enforces the provided account uuid to be empty or a valid uuid.

@kulmann
Copy link
Contributor

kulmann commented Aug 12, 2020

Validation for uuid is here:

validation.Field(&req.Identifier.AccountUuid, is.UUID),

We'd need to change that (and adjust tests) to allow alphanumeric ids.

@butonic
Copy link
Member

butonic commented Aug 12, 2020

We should not enforce the userid to be a uuid. it might be provided by other means, eg. legacy systems might migrate the username and use it as the uuid to prevent federated sharing ids from breaking.

@PVince81 PVince81 mentioned this issue Aug 12, 2020
1 task
@PVince81
Copy link
Contributor Author

PVince81 commented Aug 12, 2020

PR here for validation adjustment: #44

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants