feat: exposing sessions to the api #4352

nicolasburtey · 2024-04-24T12:37:53Z

This PR is exposing the kratos/hydra sessions related to a user.
The idea is show those sessions then into the dashboard.

A follow up PR will enable the mutation to disabled other sessions.

This PR also aim at understanding how to enable long term read only sessions, something Andrew has been asking for

dolcalmi · 2024-05-01T00:32:35Z

apps/dashboard/cypress/e2e/consent-integration.cy.ts

-      cy.contains("label", "read").should("exist")
-      cy.contains("label", "read").should("be.visible")
-      cy.contains("label", "read").should("not.be.disabled")
-      cy.contains("label", "read").should("not.be.disabled")
-      cy.contains("label", "read").click()
-
-      cy.contains("label", "write").should("exist")
-      cy.contains("label", "write").should("be.visible")
-      cy.contains("label", "write").should("not.be.disabled")
-      cy.contains("label", "write").click()
-
-      cy.get("[data-testid=submit_consent_btn]").should("exist")
-      cy.get("[data-testid=submit_consent_btn]").should("be.visible")
-      cy.get("[data-testid=submit_consent_btn]").should("not.be.disabled")
-      cy.get("[data-testid=submit_consent_btn]").click()
-


why this was removed? not related to the PR

It is, we're skipping consent in the tests

https://github.com/GaloyMoney/blink/pull/4352/files#diff-837b4f391cfd25241cfd9947e05f126a3078d0b3109844600ec949dcc3dd1ebdR25

dolcalmi · 2024-05-01T00:32:55Z

apps/voucher/cypress/e2e/consent-integration.cy.ts

-      cy.contains("label", "read").should("exist")
-      cy.contains("label", "read").should("be.visible")
-      cy.contains("label", "read").should("not.be.disabled")
-      cy.contains("label", "read").should("not.be.disabled")
-      cy.contains("label", "read").click()
-
-      cy.contains("label", "write").should("exist")
-      cy.contains("label", "write").should("be.visible")
-      cy.contains("label", "write").should("not.be.disabled")
-      cy.contains("label", "write").click()
-
-      cy.get("[data-testid=submit_consent_btn]").should("exist")
-      cy.get("[data-testid=submit_consent_btn]").should("be.visible")
-      cy.get("[data-testid=submit_consent_btn]").should("not.be.disabled")
-      cy.get("[data-testid=submit_consent_btn]").click()
-


It is, we're skipping consent in the tests

https://github.com/GaloyMoney/blink/pull/4352/files#diff-837b4f391cfd25241cfd9947e05f126a3078d0b3109844600ec949dcc3dd1ebdR25

bats/core/api/user.bats

dolcalmi · 2024-05-01T00:38:47Z

core/api/package.json

@@ -161,6 +161,7 @@
    "@typescript-eslint/parser": "^7.7.0",
    "@xascode/tscpaths": "0.1.4",
    "axios-mock-adapter": "^1.22.0",
+    "child_process": "^1.0.2",


last update 8 years ago https://www.npmjs.com/package/child_process

doesn't matter? it's on dev only

dolcalmi · 2024-05-01T00:39:38Z

core/api/package.json

@@ -182,6 +183,7 @@
    "pino-pretty": "^10.3.1",
    "prettier": "^3.2.5",
    "protoc-gen-js": "^3.21.2",
+    "puppeteer": "^22.7.1",


not sure about this.. integrations tests are already flacky and now introducing a package like this that runs a headless chromium can be problematic.. is not another way to test it?

I understand your point.

I tried with axios and basically spent a day on this, and I could not make it work, so I gave up.

the issue with a OAuth2 flow is that there is a lot of context carried around different screen, whether it's via parameters over the URL or via Cookie. it's quite hard to replicate this, specially for someone not having the full context of OAuth2 in its head.

using puppeteer make it a lot more straightforward.

I suggest we could try it and revert/change if this introduce flackiness to the test suite?

dolcalmi · 2024-05-01T00:48:03Z

core/api/src/services/hydra/index.ts

+    }))
+  } catch (err) {
+    // TODO
+    console.error(err)


remove + handle errors

dolcalmi · 2024-05-01T00:50:11Z

core/api/src/services/kratos/private.ts

+    expiresAt: new Date(session?.expires_at as string),
+    issuedAt: new Date(session?.issued_at as string),


I checked the Session interface and expires_at and issued_are are already strings... cast is not necessary also can hide a crash because of conversion (Date)

it can be undefined otherwise

before I had this but it's more ugly

if (!session.expires_at) throw new MissingExpiredAtKratosError()

I think it's really an issue with the type in the underlying library. so doing a cast as string seems the best option imo

core/api/test/integration/app/user/consent-list.spec.ts

dolcalmi · 2024-05-01T00:54:47Z

core/api/test/integration/app/user/consent-list.spec.ts

+  // Navigate the page to a URL
+  await page.goto("http://localhost:3001/api/auth/signin")
+
+  screenshots && (await page.screenshot({ path: "screenshot0.png" }))


is this only help for debug? what is the purpose of take screenshots?

yup indeed to help debugging going over the different screens

dolcalmi · 2024-05-01T00:56:12Z

core/api/test/integration/app/user/consent-list.spec.ts

+    await performOAuthLogin()
+
+    const res = await consentList(userId)
+    expect(res).toEqual([


if we are only testing this then this test is not needed 🤔 or it is better to use a mock (which probably does not make sense for what service is doing)

To test/develop the API, I had to get a client authenticated through an OAuth2 flow to be able to see the result.

Open to other suggestion, but that feels like a mock would not achieve the tests here, because it's really testing the integration with hydra.

bodymindarts · 2024-05-07T14:19:16Z

core/api/src/app/users/list-sessions.ts

+  return listSessionsService(userId)
+}
+
+export const listDeleguations = async ({ userId }: { userId: UserId }) => {


typo in Deleguations

github-actions bot added core dashboard consent pay map labels Apr 24, 2024

nicolasburtey force-pushed the feat--exposing-sessions-to-the-api branch 4 times, most recently from b342e0a to 149910b Compare April 29, 2024 22:33

github-actions bot added admin-panel voucher labels Apr 30, 2024

nicolasburtey force-pushed the feat--exposing-sessions-to-the-api branch from 496b05c to 263498b Compare April 30, 2024 19:27

wip

76a110f

nicolasburtey force-pushed the feat--exposing-sessions-to-the-api branch 2 times, most recently from a8c89fb to f3c5158 Compare April 30, 2024 20:50

chore: exposing consent list

8454854

nicolasburtey force-pushed the feat--exposing-sessions-to-the-api branch from f3c5158 to 8454854 Compare April 30, 2024 21:03

feat: exposing delegation

b7c50aa

nicolasburtey force-pushed the feat--exposing-sessions-to-the-api branch from f8c0ae5 to b7c50aa Compare April 30, 2024 22:56

nicolasburtey requested review from dolcalmi and bodymindarts April 30, 2024 22:57

dolcalmi requested changes May 1, 2024

View reviewed changes

Nicolas Burtey added 3 commits May 1, 2024 15:43

fix: remove skip

efb4e86

chore: using object for param

5e1b290

chore: remove child_process

535ee7f

bodymindarts reviewed May 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: exposing sessions to the api #4352

feat: exposing sessions to the api #4352

nicolasburtey commented Apr 24, 2024 •

edited

dolcalmi May 1, 2024

nicolasburtey May 1, 2024

dolcalmi May 1, 2024

nicolasburtey May 1, 2024

dolcalmi May 1, 2024

nicolasburtey May 1, 2024

dolcalmi May 1, 2024

nicolasburtey May 1, 2024

dolcalmi May 1, 2024

dolcalmi May 1, 2024

nicolasburtey May 1, 2024 •

edited

dolcalmi May 1, 2024

nicolasburtey May 1, 2024

dolcalmi May 1, 2024

nicolasburtey May 1, 2024

bodymindarts May 7, 2024

		expiresAt: new Date(session?.expires_at as string),
		issuedAt: new Date(session?.issued_at as string),

feat: exposing sessions to the api #4352

Are you sure you want to change the base?

feat: exposing sessions to the api #4352

Conversation

nicolasburtey commented Apr 24, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolasburtey May 1, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolasburtey commented Apr 24, 2024 •

edited

nicolasburtey May 1, 2024 •

edited