deep-foundation / deeplinks

https://discord.gg/deep-foundation
The Unlicense
14 stars 9 forks source link

authorization (login) is a mess? Is DeepClient mutable or immutable? #297

Closed FreePhoenix888 closed 10 months ago

FreePhoenix888 commented 10 months ago

Description

Is our DeepClient mutable or immutable? Is it changed when we call login method or not? I have added this test: https://github.com/deep-foundation/deeplinks/blob/14c0772029ce92e1fcffaa73a035a4557075b574/tests/permissions.ts#L1128-L1161 to check whether "join" permissions sharing is working and have found out that adminDeep.linkId is changed on this line: https://github.com/deep-foundation/deeplinks/blob/14c0772029ce92e1fcffaa73a035a4557075b574/tests/permissions.ts#L1141-L1143 Why then our "official" examples always show such examples where you login and then create a new deep client using login result: https://github.com/deep-foundation/deeplinks/blob/14c0772029ce92e1fcffaa73a035a4557075b574/tests/permissions.ts#L1132-L1136 It looks like deep client is immutable but it is not!

Proposition

Make deep client either immutable or mutable. If we think it must be mutable - do not show examples that create an illusion that it is immutable

Questions

I want to login as admin, create a new user, give admin permissions to that user and have to deep clients, not only one, the first one should be admin and the second one a user. How can I do that?

Konard commented 10 months ago

This way to run the test does not work:

npm run test -- -t '"login permission can be granted"'
gitpod /workspace/dev/packages/deeplinks (main) $ npm run test -- -t '"login permission can be granted"'

> @deep-foundation/deeplinks@0.0.329 test
> npm run package:build && cross-env DEEPLINKS_HASURA_PATH=localhost:8080 DEEPLINKS_HASURA_SSL=0 DEEPLINKS_HASURA_SECRET=myadminsecretkey DOCKER_DEEPLINKS_URL=http://localhost:3006 NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.js *.js --testTimeout=50000 -t "login permission can be granted"

> @deep-foundation/deeplinks@0.0.329 package:build
> tsc --project tsconfig.json

(node:20168) ExperimentalWarning: VM Modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

 FAIL   dom  ./react.test.js
  ● Test suite failed to run

    Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

    Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.

    By default "node_modules" folder is ignored by transformers.

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
     • If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/configuration
    For information about custom transformations, see:
    https://jestjs.io/docs/code-transformation

    Details:

    /workspace/dev/node_modules/@react-hook/debounce/dist/module/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import * as React from 'react';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      at Runtime.createScriptFromCode (../../node_modules/jest-runtime/build/index.js:1505:14)

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From react.test.js.

(node:20174) ExperimentalWarning: VM Modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:20167) ExperimentalWarning: VM Modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. Active timers can also cause this, ensure that .unref() was called on them.
Test Suites: 1 failed, 2 skipped, 1 of 3 total
Tests:       210 skipped, 210 total
Snapshots:   0 total
Time:        2.263 s
Ran all test suites matching /index.js|integration.test.js|jest.config.js|react.test.js|unit.test.js|warmup.js/i with tests matching ""login permission can be granted"" in 2 projects.
npm ERR! Lifecycle script `test` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: @deep-foundation/deeplinks@0.0.329 
npm ERR!   at location: /workspace/dev/packages/deeplinks 
gitpod /workspace/dev/packages/deeplinks (main) $

@FreePhoenix888 how do you execute this test?

FreePhoenix888 commented 10 months ago

@Konard

npm run test -- --testNamePattern "login permission can be granted"
Konard commented 10 months ago

@FreePhoenix888

Screenshot_20240129_151439

FreePhoenix888 commented 10 months ago

@Konard create a new user, join to admin, insert type is the name of the test

Konard commented 10 months ago

@Konard create a new user, join to admin, insert type is the name of the test

So this command should be executed?

npm run test -- --testNamePattern "'create a new user, join to admin, insert type'"

Screenshot_20240129_151949

But there is no such test.

Looks like I didn't do

git pull
Konard commented 10 months ago

Screenshot_20240129_152207

 permissions › join › create a new user, join to admin, insert type

    DeepClient Insert Error: recursion detected for link #1270 in spreadingFlow mp #23828 outcomingFlowDown mp #23821

      749 |       const sqlError = e?.graphQLErrors?.[0]?.extensions?.internal?.error;
      750 |       if (sqlError?.message) e.message = sqlError.message;
    > 751 |       if (!this._silent(options)) throw new Error(`DeepClient Insert Error: ${e.message}`, { cause: e })
          |                                         ^
      752 |       return { ...q, data: (q)?.data?.m0?.returning, error: e };
      753 |     }   
      754 |   

      at DeepClient.<anonymous> (imports/client.tsx:751:41)
          at Generator.throw (<anonymous>)
      at rejected (imports/client.js:17:32)

@FreePhoenix888 there is an error, but you told me there is no error and this error is not noted in the description of the issue.

FreePhoenix888 commented 10 months ago

Screenshot_20240129_152207

 permissions › join › create a new user, join to admin, insert type

    DeepClient Insert Error: recursion detected for link #1270 in spreadingFlow mp #23828 outcomingFlowDown mp #23821

      749 |       const sqlError = e?.graphQLErrors?.[0]?.extensions?.internal?.error;
      750 |       if (sqlError?.message) e.message = sqlError.message;
    > 751 |       if (!this._silent(options)) throw new Error(`DeepClient Insert Error: ${e.message}`, { cause: e })
          |                                         ^
      752 |       return { ...q, data: (q)?.data?.m0?.returning, error: e };
      753 |     }   
      754 |   

      at DeepClient.<anonymous> (imports/client.tsx:751:41)
          at Generator.throw (<anonymous>)
      at rejected (imports/client.js:17:32)

@FreePhoenix888 there is an error, but you told me there is no error and this error is not noted in the description of the issue.

Yeah that is the error I get

FreePhoenix888 commented 10 months ago

You can go up and see console logs where you can see that adminDeep.linkId is changed to the newUserDeep.linkId. That is the reason. Code is trying to insert join with from and to equal to newUserDeep.linkId

Konard commented 10 months ago

@FreePhoenix888 Screenshot_20240129_152907 The problem is that you are trying to insert this link:

{ type_id: 66, from_id: 1269, to_id: 1269 }

Meaning that you're trying to create Join link from 1269 user to itself, it is not allowed by current mp implementation. And even if mp implementation did support such thing, it would be a useless action to do anyway. User does have its own permissions in any case.

FreePhoenix888 commented 10 months ago

@Konard are you kidding me or what? Return back when you have fresh mind

Konard commented 10 months ago

We decided to make deep.login function immutable again relative to DeepClient options/configuration.

Konard commented 10 months ago

Solution

      const unloginedDeep = new DeepClient({ apolloClient });
      const guestLoginResult = await unloginedDeep.guest();
      const guestDeep = new DeepClient({ deep: unloginedDeep, ...guestLoginResult });
      const adminLogin = await guestDeep.login({
        linkId: await guestDeep.id('deep', 'admin'),
      });
      console.log({ adminLogin })
      const adminDeep = new DeepClient({ deep: guestDeep, ...adminLogin });
      // console.log({ adminDeep })
      const { data: [{ id: newUserLinkId }] } = await adminDeep.insert({
        type_id: adminDeep.idLocal("@deep-foundation/core", "User")
      })

      const newUserLoginResultBeforeJoin = await unloginedDeep.login({
        linkId: newUserLinkId
      });
      console.log({ newUserLoginResultBeforeJoin });
      console.log({ newUserLoginResultBeforeJoinDecodedToken: parseJwt(newUserLoginResultBeforeJoin.token) });

      const joinInsertData = {
        type_id: adminDeep.idLocal("@deep-foundation/core", "Join"),
        from_id: newUserLinkId,
        to_id: adminDeep.linkId
      }
      console.log({ joinInsertData })
      const joinInsertResult = await adminDeep.insert(joinInsertData)
      console.log({ joinInsertResult });
      const newUserLoginResultAfterJoin = await unloginedDeep.login({
        linkId: newUserLinkId
      });
      console.log({ newUserLoginResultAfterJoin });
      console.log({ newUserLoginResultAfterJoin: parseJwt(newUserLoginResultAfterJoin.token) });
      const newUserDeep = new DeepClient({ deep: unloginedDeep, ...newUserLoginResultAfterJoin })
      // console.log({ newUserDeep })
      const insertResult = await newUserDeep.insert({
        type_id: newUserDeep.idLocal("@deep-foundation/core", "Type")
      })
      console.log({ insertResult })